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

Composer version constraints: to fix or not to fix ? #89

Closed
jrfnl opened this issue Aug 16, 2018 · 2 comments
Closed

Composer version constraints: to fix or not to fix ? #89

jrfnl opened this issue Aug 16, 2018 · 2 comments
Milestone

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented Aug 16, 2018

In PR #81, I updated the WPCS dependency to ^1.0.0:

"wp-coding-standards/wpcs": "^1.0.0",

WPCS will be using semantic versioning. In practice, this means that:

  • Patch versions will only contain bug fixes and changes which do not impact sniffing (readme updates and such).
  • Minor version can contain new features, such as new sniffs, either WPCS native ones or adding upstream ones via the ruleset.
  • Major versions can contain BC-breaks, i.e. removed support for previously deprecated behaviour and such.

Most of the repos which use YoastCS have a compose.lock file committed in the repo. For those the above is fine, as updates to the composer.lock will be managed.

A few repos - IIRC i18-module and WHIP, though possibly one or two more - don't have a committed composer.lock file. For those repos, the above version setting could cause unexpected build breaking when a new (minor) version of WPCS has been released and Travis would use the latest version when doing a composer install and the subsequent phpcs run.

So my question is:
👉 Should the WPCS dependency in YoastCS be limited to patch versions ? i.e. use ~1.0.0 instead.

The same could, of course, be said about the PHP_CodeSniffer and PHPCompatibilityWP dependencies.
For those, I strongly suggest leaving the dependency setting as is, i.e. ^#.#.

  • For PHP_CodeSniffer: while higher versions of PHPCS may include new sniffs, those sniffs will not automatically be included as that is managed by WPCS. So allowing higher versions should be fine as these will contain bug fixes which the projects benefit from.
  • For PHPCompatibilityWP: while higher minors will contain new sniffs, if this leads to breaking builds, that's a good thing as it generally means that a cross-version compatibility bug in the code has been discovered which should be fixed with high priority anyway.
@jrfnl jrfnl added this to the 1.0.0 milestone Aug 16, 2018
@atimmer
Copy link
Contributor

atimmer commented Aug 22, 2018

I would say go for ^1.0.0.

@jrfnl
Copy link
Collaborator Author

jrfnl commented Aug 23, 2018

@atimmer Thanks for your response. In that case, I'll close this issue, but at least the reasoning and the decision has now been documented.

@jrfnl jrfnl closed this as completed Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants