-
Notifications
You must be signed in to change notification settings - Fork 52
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
chore(deps): require slevomat/coding-standard v8 and squizlabs/php_codesniffer v3.7 #264
Conversation
simPod
commented
Jun 18, 2022
•
edited
Loading
edited
- Bump min php to 7.2
6d85421
to
5c638da
Compare
This should be reverted. One should not rely on a dependency being transitively installed when referencing things defined in it, and we do: coding-standard/lib/Doctrine/ruleset.xml Line 17 in f86c16a
Why? |
@greg0ire ad 10.0.x: this is a BC break https://github.com/doctrine/coding-standard/pull/264/files#diff-d9e16eb577617c038e05ad72a5a3dd5f93f23fd798bf4b35d668b173a9c5733bR137 sniff got renamed |
How is it a BC-break? That sniff is not part of the public API of |
I though of a case when user e.g. excluded that sniff by a name. The sniff is enabled by Doctrine CS. Not sure if it's considered BC break or not or what's actually a public api in the context of CS. 🤷🏿♀️ Ur call. |
If that name is defined by
I think it this case, the public API is the name |
If I may weigh in on the target version discussion: If this change does not justify a major version bump (10.0.0), it certainly deserves a minor version bump (9.1.0) because I believe this is not just a bugfix. |
…desniffer v3.7 - Bump min php to 7.2
I think you're correct, so I should a create a 9.1.x branch so that this can be retargeted. Looking at 9.0.0...9.0.x though, I see a couple PRs we all reviewed (including myself 😅 ) have been targeted at the wrong branch (IMO): I say we merge this one in 9.0.x as well, then merge 9.0.x up into 10.0.x, and release 10.0.0, and then we try to stick to semver 😄 I can contribute more details to https://www.doctrine-project.org/projects/doctrine-coding-standard/en/8.2/reference/index.html#versioning but I think both PRs above are already covered by that paragraph. |
With v10 lets wait I have few PRs to add |
Thanks @simPod ! |