wp-cli / scaffold-command

Generates code for post types, taxonomies, blocks, plugins, child themes, etc.
MIT License
165 stars 87 forks source link

PHPCS template: fix XML error in ruleset #171

Closed jrfnl closed 6 years ago

jrfnl commented 6 years ago

As reported by @dimadin on Slack, when using the current .phpcs.xml.dist ruleset, a phpcs: Ruleset <path>/.phpcs.xml.dist is not valid error is thrown.

This was caused by an unclosed comment in the XML. This PR fixes it.

The PR can be tested by:

  1. Save the current ruleset locally and try to use it to see the error.
  2. Save the ruleset from this PR and try again. The ruleset error will be gone.
jrfnl commented 6 years ago

Side-note: to prevent this issue in the future, I'd be happy to add XML validation to the Travis script for all XML files ?

swissspidy commented 6 years ago

@jrfnl That would be interesting. What did you have in mind for this? The only thing that comes to mind is xmllint.

jrfnl commented 6 years ago

@jrfnl That would be interesting. What did you have in mind for this? The only thing that comes to mind is xmllint.

@swissspidy That's how I've implemented it a while ago in WPCS - both validation as well as an XML codestyle check. See: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/eccb1f721f95fdcf5c2e383340aa4b094c0317ad/.travis.yml#L112-L122

schlessera commented 6 years ago

@jrfnl Linting the XML sounds good. However, this is currently a template file, so it might not pass the linter because of that. What I had been thinking recently is to let Travis actually use the scaffolding command to scaffold a plugin/theme, and then run tests against that scaffolded result.

I'll open a separate issue to that regard.

Thanks for this PR, @jrfnl !

jrfnl commented 6 years ago

However, this is currently a template file, so it might not pass the linter because of that.

It actually does pass the linter as it is now this PR has been merged, so that's not an issue.

What I had been thinking recently is to let Travis actually use the scaffolding command to scaffold a plugin/theme, and then run tests against that scaffolded result.

That sounds interesting ;-)

I haven't used the scaffold command myself yet, so this may already be covered, but in case it's not: it may also be useful to adjust the values for some of the customizable properties in the XML ruleset when doing the scaffolding.