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

Update PHP Standards. #82

Merged
merged 4 commits into from
Mar 18, 2019
Merged

Update PHP Standards. #82

merged 4 commits into from
Mar 18, 2019

Conversation

tfrommen
Copy link
Contributor

@tfrommen tfrommen commented Aug 20, 2018

This PR includes:

  • update WPCS to v1;
  • improve/fix ruleset descriptions;
  • adapt ruleset to updated standards.

Also, the ruleset has been improved:

  • specify current directory as default path (which can be overriden via command line);
  • do not exlude *.css and *.js files, but really restrict PHPCS to PHP files only.

This PR also includes #31.

Resolves #108

@tfrommen tfrommen requested a review from rmccue August 28, 2018 18:41
@ntwb
Copy link
Member

ntwb commented Oct 11, 2018

It'd be great to land this, without it I can't update WPCS to 1.0 locally as Composer SemVer requires HM 0.4.2 or 0.5.0 to use WPCS 0.14.0 /shrug

@mikeselander mikeselander added this to the 0.6 milestone Dec 18, 2018
composer.json Outdated
"fig-r/psr2r-sniffer": "^0.5.0",
"squizlabs/php_codesniffer": "~3.2.0"
"squizlabs/php_codesniffer": "~3.2.0",
"wp-coding-standards/wpcs": "^1.0.0"
Copy link
Contributor

@mikeselander mikeselander Dec 18, 2018

Choose a reason for hiding this comment

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

Might as well use 1.2.1 at this point. ignore me 😬

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 know to what degree things have changed in 1.0.0? I don't really follow wpcs at all, but I know there's been a lot of churn, and frankly adding of pretty arbitrary "standards", so this just makes me a bit nervous. If that's all gravy, and we feel we know what we are getting ourselves in for, no worries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joehoyle from 0.x to 1.0.0, there did change a lot, most of it being re-structuring of rules, plus a couple deprecations here and there... In this PR, I pretty much made the resulting rules be equivalent to what we had before.

But, to be fair, this PR is almost half a year old, and WPCS is already at version 2.0.0 at the moment. Just saying...

HM/ruleset.xml Show resolved Hide resolved
@nathanielks
Copy link

This has been approved, can it be merged?

@mikeselander mikeselander mentioned this pull request Feb 15, 2019
composer.json Outdated
"fig-r/psr2r-sniffer": "^0.5.0",
"squizlabs/php_codesniffer": "~3.2.0"
"squizlabs/php_codesniffer": "~3.2.0",
"wp-coding-standards/wpcs": "^1.0.0"
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 know to what degree things have changed in 1.0.0? I don't really follow wpcs at all, but I know there's been a lot of churn, and frankly adding of pretty arbitrary "standards", so this just makes me a bit nervous. If that's all gravy, and we feel we know what we are getting ourselves in for, no worries.

HM/ruleset.xml Show resolved Hide resolved
@ntwb
Copy link
Member

ntwb commented Feb 19, 2019

@joehoyle wrote:

Do we know to what degree things have changed in 1.0.0? I don't really follow wpcs at all, but I know there's been a lot of churn, and frankly adding of pretty arbitrary "standards", so this just makes me a bit nervous. If that's all gravy, and we feel we know what we are getting ourselves in for, no worries.

FWIW, WPCS 2.0.0 has since been released:

https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases/tag/2.0.0

@mikeselander
Copy link
Contributor

mikeselander commented Mar 18, 2019

I'm going to merge this and we will be dogfooding it immediately on a client project. If any updates are needed we should know quickly and before we tag 0.6.

Note: CI isn't passing but I will fix that this week.

@mikeselander mikeselander merged commit 8d8f1df into master Mar 18, 2019
@mikeselander mikeselander deleted the update-php-standards branch March 18, 2019 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NoSpaceAfterCloseParenthesis is (currently) inconsistent.
6 participants