Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring template file from wp-cli #73

Merged
merged 3 commits into from Jun 3, 2018

Conversation

2 participants
@sasagar
Copy link
Contributor

sasagar commented Jun 3, 2018

Related #72

I'm not sure this PR is enough for the issue, but the test has been passed.
If not, I'm happy to re-commit for that.

(This is my first PR for the code at WordCamp Osaka 2018.)

Thanks.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Jun 3, 2018

@sasagar Thanks for the pull request (and say hello to everyone at the CLI table in Osaka from me)!

The pull request is not correct yet. First of all, the template should go into a templates subfolder, to stick to current conventions.

Also, regarding pulling in the template via the make-phar tool:
This does not happen within this package, but is part of the overarching wp-cli/wp-cli package. So for this PR, you can just remove the make-phar bit again and it is good to be merged.

If you want to work on the makephar bit, you have to do this in the wp-cli/wp-cli package and produce a pull request against that one. Here's an example of how the templates for the config-command are pulled in: https://github.com/wp-cli/wp-cli/blob/master/utils/make-phar.php#L255-L264

You can just copy this and adapt it for the core-command.

@@ -0,0 +1,2 @@
<?php
include( '../version.mustache' );

This comment has been minimized.

@schlessera

schlessera Jun 3, 2018

Member

This is not needed here and should be done in the wp-cli/wp-cli package instead.

@@ -0,0 +1,4 @@
WordPress version: {{wp-version}}

This comment has been minimized.

@schlessera

schlessera Jun 3, 2018

Member

Please put this file into a templates subfolder to stick with current conventions.

@@ -0,0 +1,4 @@
WordPress version: {{wp-version}}

This comment has been minimized.

@schlessera

schlessera Jun 3, 2018

Member

Ha, almost!

The folder is called templates (plural), not the singular template.

This comment has been minimized.

@sasagar

sasagar Jun 3, 2018

Author Contributor

Oops, I'd like to change it!

@schlessera schlessera added this to the 1.0.10 milestone Jun 3, 2018

@schlessera schlessera merged commit d557fb4 into wp-cli:master Jun 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sasagar sasagar deleted the sasagar:develop-mustache branch Jun 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.