Rebuild coding standards ruleset and eliminate sniffs in favor of standards#6956
Rebuild coding standards ruleset and eliminate sniffs in favor of standards#6956wilsonge merged 19 commits intojoomla:stagingfrom
Conversation
|
@zero-24 Should this PR get the "Code Style" label too? |
|
@photodude this has the label already 😄 |
|
@zero-24 I see |
|
Woops my eyes only read what they expect 😄 To answer your question
We label PRs that fix only codestyle issues with |
|
@zero-24 Thanks for the clarification |
|
Thanks very much @photodude for working on this 😄 |
81fc691 to
fe0a85c
Compare
|
@javigomez you're welcome. It looked like a good place to get my feet really wet with the the project. Still a lot to do to our custom sniffs up to get up to speed and running with PHPCS-2. So far the custom commenting sniffs are still failing in my test branch, https://github.com/photodude/joomla-cms/tree/PHPCS-2 |
c398665 to
5151bdd
Compare
592078c to
bab685f
Compare
ab7113d to
c4e7c7e
Compare
|
I have rebased this PR to the current Stagging and fixed the merge conflicts so this can be reconsidered. The PHPCS checks are failing since this PR is more accurately finding the code style issues. As such, it looks like I need to adjust more of the exclusion rules to temporarily account for the existing failures. |
|
@javigomez @zero-24 @mbabker @wilsonge This PR should now be in a state ready for review and consideration. |
|
Can you ping me on this as soon as 3.5 ships. Moving to PHPCS 2 is rapidly rising up my priority list and would love to get this into 3.5.1! |
|
Sure, But I do want to be clear this PR doesn't make the move to PHPCS 2 yet. This just cleans up the existing PHPCS 1.5 standards following what was done for the draft of the coding standards for PHPCS 2. There is a Draft for the full Joomla Coding standard running PHPCS 2, it does need some additional work to fix some consistency issues and it is composer compatible . So Ideally the Draft for the full Joomla Coding standard running PHPCS 2 should be completed and then brought over to the CMS either directly or via composer |
|
Just on a quick glance, things seem fine here. 👍 |
|
Can we get this merged? @wilsonge its not something that can really be tested at the CMS level This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6956. |
|
@photodude can you fix the conflicts please? Let's get this show on the road :) |
|
@wilsonge Is it worth fixing this PR again? Would it be better to just put the effort into finishing the PR for the full Joomla Coding standard running PHPCS2 and bring that in with composer? |
- Note: some new sniffs were added from the Joomla Coding standards (used standards rather than the Joomla sniffs since there is no difference from the source)
Removing Squiz.WhiteSpace.MemberVarSpacing since that standard requires an empty line after the opening class bracket `{`
Add back the Joomla.WhiteSpace.MemberVarSpacing sniff
Needed due to the removal of Squiz.WhiteSpace.MemberVarSpacing sniff since that standard requires an empty line after the opening class bracket `{`
PEAR.WhiteSpace.ObjectOperatorIndent is calculating the space oddly resulting in false positive errors Replace by adding back Joomla.WhiteSpace.ObjectOperatorIndent Reorder the Joomla standards list to alphabetical sniff ordering
Removed PEAR.WhiteSpace.ObjectOperatorIndent as it is calculating the space oddly resulting in false positive errors
exceptions added since we are catching the errors now. Expected temporary, possibly permanent.
Add exceptions for EndFileNewline since we are actually catching the errors now. These exceptions are temporary for now, possibly permanent
Squiz.Commenting.DocCommentAlignment seems to be giving a lot of errors which seem to be false positives vs the Joomla Coding standards. Assuming this is covered by the other commenting sniffs
Actually remove rather than just adding exclusions.
|
@wilsonge I have fixed the merge conflicts. This PHPCS1.5.x improvement is again ready for consideration. I do feel that effort should go into finishing the PR for running PHPCS2 and bring that in with composer. (The Full PHPCS2 version mostly needs testing and validation of the fixers) In any case, this PR is an improvement in the PHPCS1.5.x code standards for the CMS, and will be beneficial until the PHPCS2.x version is ready. |
|
At a quick look over this looks good. But going to go through over the weekend to assure myself we aren't loosing any checks here :) |
|
Was easier than i thought to go through everything. Nice work :) Look forward to sitting down and trying to sort out v2 |
This brings in some of the changes from the Joomla coding standards
and some of suggested changes from the Joomla coding standard
A number of temporary exclude patterns were added following the pattern currently established, these should be eliminated as the coding standard deviations are corrected.
These changes build a foundation to facilitate a move to PHPCS 2.x