User tests: Successful: Unsuccessful:
Pull Request for Issue code style corrections for doc comment types. continues work from #20522
PHPCS2 manual fixes
&
prefixed in the doc comment
Found using the Joomla code standards 2.0.0 PHPCS2-RC
Merge by code review
code style has been applied as listed above, old code style testing on drone does not error.
code style had not been applied. Found using the Joomla code standards 2.0.0 PHPCS2-RC
none
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_finder com_installer com_menus Repository |
Labels |
Added:
?
|
I have tested this item
@laoneo The original plan was to get 3.x on PHPCS2 which requires cleaning up some code style items to make sure everything is working correctly, then work on getting J4 running on PHPCS3+. see the last few comments of joomla/coding-standards#143
We have found an issue with anonymous classes and closures indenting that needs to be addressed before we can finalize either of the updates. see joomla/coding-standards#230
We could consider large execution lists for both J3 and J4 to implement the updates now, but it increases the probability that the exclusions become permanent and somethings will not be up to the code standards.
Regardless of whether these are merged to staging or 4.0-dev it still creates merge conflicts. It's not like merging to only one branch is going to magically make things easier.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-06-08 18:26:20 |
Closed_By | ⇒ | zero-24 | |
Labels |
Added:
?
|
Merged
Thanks Everyone
By the same argument then the privacy tool suite should only merge to 4.0 because THAT is going to create all sorts of merge conflicts too.
Again, it doesn't matter what branch these things land on, they're going to create merge conflicts somewhere and make someone's life miserable. There is no getting around that without hard forking the 4.0 branch (almost the same way 2.5 and 3.x were dealt with after 3.0's release) and not merging up ANY changes from any lower branches.
Not sure how a new feature can add that many conflicts. Anyway. What I wanted to express here is that all I try is to organize things a bit for J4 and I see I fail once more. That we are since months over 250 commits behind staging documents our current situation very well.
Don't get me wrong, the CS fixer work is important and I appreciate all what you guys are doing.
I mean you can't blame the project to be an unorganized mess and then on the other hand stop people to work against that. But I see it turned out once more in a nonsense argumentation battle. I will just shut up as I just don't have time for.
No drama intended.
The feature creates conflicts in common integration points. Ask anyone who's done work on that repo and had to deal with constantly changing SQL files, that's just one feature repo. Add the SQL changes in 4.0.
As for CS fixes, here's the thing. If you ONLY apply them to 4.0, sooner or later it actually makes dealing with merges more difficult because the code starts diverging so much from the lower branches. If you apply them on lower branches and merge up, sure you're going to run into merge conflicts in that initial merge, but it shouldn't create too many more problems after that unless you're constantly changing those same blocks of code.
I did a lot of CS changes in the Framework's 2.0 branches to adapt to PHP 7 syntactic sugar that wasn't available in PHP 5.3, then had the big move of all the packages to the PHPCS 2 standards. In quite a few packages, branch merging is not a simple thing because of that (especially something like the Filesystem package where we basically changed every snake case variable to camel case in one go). So in the long run, it's more sane for everyone to keep the code structure/style consistent across branches, even if it means there are short term complications. It's already annoying enough having to unconflict 7 or 8 files every time we bump version strings, and nobody likes the "new year new copyright statement" commit.
Every pr which can be moved to J4 creates no conflicts at all. I get your argument about the diverge, but when we can change the pr's which are not really needed (I hope this understands now nobody the wrong way) to J4, then I would say it rather motivates people in participating in J4. Just wondering how @wilsonge feels about all of this as he is most affected because he manages the sync.
@mbabker @wilsonge @zero-24 @Quy
For easy review purposes, and to avoid large scale merge conflicts, I've been trying to keep these PR's between 15-25 files; Please let me know if you would like to see PR's with more files to help move the PHPCS 2 update project along faster.
With the current modified exclusion list I'm working from I have
A TOTAL OF 809 ERRORS AND 1 WARNING WERE FOUND IN 265 FILES
wherePHPCBF CAN FIX 169 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
output.txt