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

CS: fix compliance with PSR12 #88

Merged
merged 1 commit into from
Dec 14, 2019
Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 8, 2019

Proposed Changes

PHPCS 3.5.0+ contains a far more complete PSR12 ruleset.

  1. The current code didn't fully comply with that ruleset. Some minor code tweaks fix that.
  2. PSR12 demands constant visibility to be declared, but as this project still supports PHP 5.3 and constant visibility wasn't introduced until PHP 7.1, this is not desirable. Excluding that particular rule from within the ruleset fixes that.

Related Issues

PHPCS 3.5.0+ contains a far more complete PSR12 ruleset.

1. The current code didn't fully comply with that ruleset. Some minor code tweaks fix that.
2. PSR12 demands constant visibility to be declared, but as this project still supports PHP 5.3 and constant visibility wasn't introduced until PHP 7.1, this is not desirable. Excluding that particular rule from within the ruleset fixes that.
Copy link
Contributor

@mjrider mjrider left a comment

Choose a reason for hiding this comment

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

lgtm

@mjrider mjrider merged commit 6f35da5 into master Dec 14, 2019
@jrfnl jrfnl deleted the feature/update-phpcs-ruleset branch December 14, 2019 20:31
@jrfnl
Copy link
Member Author

jrfnl commented Dec 14, 2019

@mjrider Thanks. Now we have a passing build again, I'll be sending in some more PRs to further improve the testing of the plugin. I'm sending them in staged as otherwise we'd have to fix conflicts all the time.

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.

3 participants