Skip to content

Scaffold cpt / tax with label - #365

Merged
scribu merged 6 commits into
wp-cli:masterfrom
linepress:scaffold-label
Apr 5, 2013
Merged

scribu merged 6 commits into
wp-cli:masterfrom
linepress:scaffold-label

Conversation

@jmslbam

@jmslbam jmslbam commented Mar 27, 2013

Copy link
Copy Markdown
Contributor

See #364
Have put the label next to the text-domain.

Comment thread php/commands/scaffold.php Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The preg_replace() should go in the extract_args() array above, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Don't really see how that would be implemented, technically :)
The reason I did this is because the $slug is the default value, and the $slug also needs the preg_replace(). So doing this after the whole extract_args part, keeps it down to 1x preg_replace, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The idea is that you should do the preg_replace() only if the slug is used. Since it isn't very expensive, you can just do:

array(
  'label' => preg_replace( '/_|-/', ' ', strtolower( $slug ) ),
  ...
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, because when there is a _ or a - in the --label, it is put there deliberate.

@scribu

scribu commented Mar 27, 2013

Copy link
Copy Markdown
Member

Also, where are the functional tests? :D

@jmslbam

jmslbam commented Mar 27, 2013

Copy link
Copy Markdown
Contributor Author

:) here #366? Not the right command though haha

@scribu

scribu commented Mar 27, 2013

Copy link
Copy Markdown
Member

You could create a new scaffold.feature file which just tests that STDOUT contains whatever you passed as --label.

@jmslbam

jmslbam commented Mar 27, 2013

Copy link
Copy Markdown
Contributor Author

Cool, but in this pull?
Wanted to make a different branch for that.

On Wed, Mar 27, 2013 at 9:42 PM, Cristi Burcă notifications@github.comwrote:

You could create a new scaffold.feature file which just tests that STDOUT
contains whatever you passed as --label.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/365#issuecomment-15551819
.

@scribu

scribu commented Mar 27, 2013

Copy link
Copy Markdown
Member

Yes, in this pull, because this pull contains the functionality that you're testing.

Comment thread features/scaffold.feature Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tags shouldn't contain spaces and should be lowercase, so the correct name would be @custom-post-types.

Also, I'm pretty sure you would have to add it for each scenario.

With that said, I don't really see the usefulness in tags at this point, since tests are already pretty well grouped by file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought that at some point the scaffolding tests would grow and grow because it houses quite some functionality:

  • cpt
  • tax
  • plugin (tests)
  • _s theme

So when editing a test, one could filter run the behat test --tag=cpt so it doesnt run all the tests.
Off course, the a final check should be ran on the complete feature. But in development, a tag could come in handy.

Anyway, updated the PR :)

P.s. You're right about the spelling:
http://docs.behat.org/guides/3.hooks.html#tagged-hooks
http://docs.behat.org/guides/6.cli.html#gherkin-filters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought that at some point the scaffolding tests would grow and grow because it houses quite some functionality

That's a good point, but let's cross that bridge when we reach it.

scribu pushed a commit that referenced this pull request Apr 5, 2013
@scribu
scribu merged commit 4c476fa into wp-cli:master Apr 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants