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

Plan for the migration to PHPCS 3 #80

Closed
stof opened this issue May 31, 2017 · 15 comments
Closed

Plan for the migration to PHPCS 3 #80

stof opened this issue May 31, 2017 · 15 comments

Comments

@stof
Copy link
Contributor

stof commented May 31, 2017

PHPCS 3 was just released, with BC breaks for custom sniffs. what are the plans to migrate to it ?

@Telematica
Copy link

Telematica commented May 31, 2017

@stof +1 👍
Had to downgrade PHPCS 3 to 2.9.x to get Symfony2 CS working properly.
Any plans to port/migrate to PHPCS 3

@KacerCZ
Copy link

KacerCZ commented Jun 1, 2017

Migration is in the works - see #64 and #63

@wickedOne
Copy link
Contributor

the current master branch is phpcs3 compatible while a 2.x branch has been created for the continuos support of phpcs2.

i'm unaware of the release strategy of @djoos, but for now i think it's easiest to lock to 1888917 which is the current 2.x branch made phpcs3 compatible so should behave exactly the same.

in the master branch there have been some additional changes like changing the standard's name and the addition of 7 new sniffs (3 still open in pr) in an attempt to comply with symfony's current standards as described in #66, #68 and #69.

as those new sniffs are only tested on my own repositories and the past has shown there are plenty of (edge) cases resulting in false positives, those might need some additional testing and finetuning.

@djoos
Copy link
Owner

djoos commented Jun 7, 2017

v3.0.0 of the CS is in the works and will get tagged as soon as we're ready. For now PHPCS3-users should be able to make use of the master of this repo, but it's still a WIP. Comments and remarks always welcome via separate issues/PRs!

I don't think we'll backport any of the new sniffs into the 2.x branch though, however I'm open for a discussion on that.

Hope this helps!

@kevinvanrijn
Copy link

From what I can tell, #70 fixed a regression from upgrading to PHPCS 3.

So, should I be using b097f90 rather than 1888917 to keep the same behavior after upgrading?

@djoos
Copy link
Owner

djoos commented Jun 13, 2017

@KIPdeKIP: I'd stick to the latest master until we properly release 3.0.0. Hope this helps!

@wickedOne
Copy link
Contributor

@KIPdeKIP indeed 1888917 has some regressions due to unexpected behaviour of phpcs3 (not sure whether it's a bug or intended behaviour) which is very unfortunate...

so to mimic the exact behaviour of the v2 branch with phpcs v3 you need to patch 1888917 and cherry pick it with b097f90 and 8d5d452

@djoos it might be an idea to do the above in a seperate branch and tag it so people can stick to that until the 3.0.0 release? meanwhile i'll open an issue and investigate why the unittests stopped working (which would have prevented this from happening)

@kevinvanrijn
Copy link

Instead of 8d5d452 did you mean cf5a004? 8d5d452 is giving me a "fatal: bad object" error. Did the history get rewritten at some point?

What I ended up doing is rebase picking b097f90 and cf5a004. Dropping everything else after 1888917.

Fixed the conflicts caused by the rename from Symfony2 to Symfony and pushed them to my fork. Does this look correct? Everything seems to be working perfectly.

If it is decided to create a seperate branch feel free to fork mine back or create your own.


Thanks for all the help. Hope to see 3.0.0 release soon.

@wickedOne
Copy link
Contributor

yes, you're right: inbetween those commits we've changed the namespace of the sniffs and thus the rules in the ruleset.xml sorry for not catching that before making my comment yesterday...

your branch looks to be the v3 version of v2 of what i was trying to describe!

thanks for noticing!

and as you appear to be using the standard: if it's not too much time and effort for you, it would be highly appreciated if you're willing to run the current master branch of this repo against your codebase(s) and will let us know whether the new sniffs raise any false positives. so we can fix those.

as said earlier in this discussion: all of them are merely tested against the repositories i currently work with and djoos' so any input is welcomed!

@wickedOne
Copy link
Contributor

@djoos as #69, #68 and #66 are resolved i personally think #92 is the only thing standing in the way to create a v3 version and go forward from that.

do you have any release strategy for v3 or any additions for us to get to a v3?

p.s. feel free to contact me directly through email

@djoos
Copy link
Owner

djoos commented Jun 23, 2017

Hi @wickedOne,

after #92, I'm happy to go ahead and release v3. Thank you for the amazing work you put into the PHPCS3 migration!

I'll close this issue for now, however: give a yell if anyone has other objections to getting 3.0.0 out. Thanks in advance!

@djoos djoos closed this as completed Jun 23, 2017
@wickedOne
Copy link
Contributor

@djoos thanks & you're welcome: more than happy to give back to the community i pretty much based my carreer on... 😎

i'll have a look @ #92 somewhere next week and guess we have to grab the false positives from each ci locked to dev-master :-)

@mnapoli
Copy link

mnapoli commented Aug 31, 2017

@djoos Hi, could you tag a 3.0 version?

Thanks!

@djoos
Copy link
Owner

djoos commented Sep 1, 2017

3.0.0 has been tagged, I'm aware we still have a few issues outstanding, but happy to get those out in minor releases... HTH!

@mnapoli
Copy link

mnapoli commented Sep 1, 2017

Awesome thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants