Skip to content

Add clarification to Getting Started docs#2040

Merged
jarednova merged 2 commits intotimber:masterfrom
joepetrakovich:master
Jul 4, 2019
Merged

Add clarification to Getting Started docs#2040
jarednova merged 2 commits intotimber:masterfrom
joepetrakovich:master

Conversation

@joepetrakovich
Copy link
Copy Markdown
Contributor

Clarified which directory Composer should be used in. Also added clarification for the scenario of using Composer with the starter theme instead of the WordPress.org install.

Ticket: #2039

Issue

Trying to use both the Composer install instructions and the starter theme instructions resulted in some confusion when it came to activating the Timber plugin.

Solution

Added clarification for that specific use case.

Impact

Possibly less headaches from new users, less support tickets.

Considerations

It's odd that the starter theme has a composer.json file that is using an older Timber version and uses that package type that installs it as a plugin rather than the Composer autoload mechanism. I'm not really sure the best solution.

Clarified which directory Composer should be used in.  Also added clarification for the scenario of using Composer with the starter theme instead of the WordPress.org install.
@joepetrakovich joepetrakovich requested a review from gchtr as a code owner July 1, 2019 23:14
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 1, 2019

Codecov Report

Merging #2040 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2040   +/-   ##
=========================================
  Coverage     95.01%   95.01%           
  Complexity     1570     1570           
=========================================
  Files            49       49           
  Lines          3710     3710           
=========================================
  Hits           3525     3525           
  Misses          185      185

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78e5cd0...5812d84. Read the comment docs.

@palmiak
Copy link
Copy Markdown
Collaborator

palmiak commented Jul 2, 2019

I'm wondering shouldn't we just make a change in the starter theme so it would use timber as library not as a plugin.

Still the documentation here needs 3 variants plugin, lib via composer and plugin via composer. So the changes you made @joepetrakovich should still be included.

@gchtr gchtr requested a review from jarednova July 2, 2019 06:05
Copy link
Copy Markdown
Member

@gchtr gchtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joepetrakovich! Your additions will make it much clearer how things work 👍.

I agree with @palmiak, we should probably update how the Starter Theme is installed. There’s already an open issue that tracks this.

@jarednova I’ll let you do the final check on this, because you’re the Starter Theme master.

@joepetrakovich
Copy link
Copy Markdown
Contributor Author

joepetrakovich commented Jul 2, 2019

You're welcome, and yes I also agree the starter theme should be consistent.

What about simply removing the composer.json from it? That way it would be more clear that you ought to just use the WordPress.org plugin with it, and if not, it would be like any other theme that would need modification to the functions.php. That would reduce the number of explanation variants from 3 to 2.

Unless it's better to have a locked Timber version to go with that theme.

@jarednova jarednova self-assigned this Jul 3, 2019
@jarednova
Copy link
Copy Markdown
Member

jarednova commented Jul 4, 2019

I think you're on the right track @joepetrakovich. I still want to make the starter theme versatile to use in either setup. But the composer.json file should anticipate people using via Composer instead of as the WP plugin. Additionally, I added the block to load the Composer autoload so we can still reduce the number of explanation variants from 3 to 2.

@timber timber deleted a comment from coveralls Jul 4, 2019
@timber timber deleted a comment from coveralls Jul 4, 2019
@jarednova jarednova merged commit 773f9db into timber:master Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants