Skip to content

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Sep 17, 2025

Closes #345

Resolves #263

@simPod simPod force-pushed the squiz-4 branch 8 times, most recently from f67256b to f1d2d7f Compare September 17, 2025 18:48
@simPod simPod marked this pull request as ready for review September 17, 2025 18:49
@simPod simPod requested a review from a team as a code owner September 17, 2025 18:49
ostrolucky
ostrolucky previously approved these changes Sep 17, 2025
SenseException
SenseException previously approved these changes Sep 17, 2025
"slevomat/coding-standard": "^8.16",
"squizlabs/php_codesniffer": "^3.7"
"slevomat/coding-standard": "^8.23",
"squizlabs/php_codesniffer": "^4"
Copy link
Member

Choose a reason for hiding this comment

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

Do we have make a hard bump here or could we support both major versions at the same time?

Copy link
Contributor

Choose a reason for hiding this comment

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

slevomat/coding-standard force the 4.0 too (in the 8.23). I may be possible to support both versions but there are some internal changes in PHPCS and it requires more work than I was willing to put into it…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E.g. this definition is not v3/v4 cross compatible.

image

And also what @kukulich said.

Copy link
Contributor

Choose a reason for hiding this comment

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

@simPod Eh... sorry, but no. What's on the right there has been supported by PHPCS since a very very long time, so definitely cross-version compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, that I missed. But still, this CS relies heavily on Slevomat's CS which requires v4 so allowing v3 here has no point if I'm not missing anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I saw that and respect @kukulich's decision in that.
In most cases, making sniffs PHPCS 3/4 cross-version compatibility isn't necessarily that hard, though yes, it does mean more work/higher cognitive load for a period of time and I totally get it if someone maintaining an external standard does not want to have to deal with that.

For the record: PHPCSUtils, PHPCSExtra and PHPCompatibility have all been made cross-version compatible.

@greg0ire greg0ire merged commit 897a7dc into doctrine:14.0.x Sep 21, 2025
13 checks passed
@greg0ire
Copy link
Member

Thanks @simPod !

@simPod simPod deleted the squiz-4 branch September 21, 2025 19:03
@greg0ire greg0ire added this to the 14.0.0 milestone Sep 22, 2025
@greg0ire
Copy link
Member

@simPod sorry for not asking earlier, but do you know the breaking changes are exactly? Are there new sniffs? Or will this maybe break some CIs somehow?

@simPod
Copy link
Contributor Author

simPod commented Sep 22, 2025

Sure. I went through the upgrade guide and didn’t find anything that would significantly impact end users.

From what I understand, the v4 release is mainly a cleanup and consolidation effort—removing legacy code and streamlining things. It seems to primarily affect sniff developers, while regular PHPCS users with clean configurations shouldn’t notice much change. I even did not have to touch tests here.

(maybe Juliette can elaborate)

@greg0ire
Copy link
Member

Oh OK, then I guess it's not worth publishing 14.0.0 yet?

@simPod
Copy link
Contributor Author

simPod commented Sep 22, 2025

IMO yes cuz otherwise people would be blocked from upgrading Slevomat CS to further versions (new sniffs and fixes) 🙃.

E.g. I require both Doctrine CS and Slevomat CS. But Doctrine's one now blocks me from upgrading to Squizlab's v4 and thus upgrading Slevomat's.

@jrfnl
Copy link
Contributor

jrfnl commented Sep 22, 2025

From what I understand, the v4 release is mainly a cleanup and consolidation effort—removing legacy code and streamlining things. It seems to primarily affect sniff developers, while regular PHPCS users with clean configurations shouldn’t notice much change. I even did not have to touch tests here.

That's correct and anything I could think of that would impact end/dev users is documented in the upgrade guides:

Aside from that, there is also the changelog of course:

The reason I made PHPCSUtils, PHPCSExtra and PHPCompatibility cross-version compatible is to allow for end-users which use a combination of external standards, some of which may be ready for 4.x and some of which may not.

If your user-base is known to often combine multiple external standards, making the package PHPCS cross-version compatible may be preferred to allow users to upgrade [to PHPCS 4.x] as soon as possible, while still benefiting from updates to your package if they can't upgrade to PHPCS 4.x yet.

Also see: https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Version-4.0-Developer-Upgrade-Guide#should-i-upgrade-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants