-
Notifications
You must be signed in to change notification settings - Fork 40
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
Rulesets: move VariableAnalysis configuration from Go to Minimum #763
Conversation
While looking into 588, I noticed something which seemed odd, so I'm addressing this with this PR. The configuration which was added in 690 (and addressed 588) was added to the `WordPress-VIP-Go` ruleset, not the `WordPressVIPMinimum` ruleset from which the `WordPress-VIP-Go` ruleset inherits. I can not think of any pertinent reason why the configuration should be added for one, but not the other and I couldn't find any discussion on this either, so I propose moving the configuration from the `WordPress-VIP-Go` ruleset to the `WordPressVIPMinimum` ruleset. Ref: https://github.com/sirbrillig/phpcs-variable-analysis/releases/tag/v2.9.0
Nobody should be using What advantage does having it in the If the name of the |
Well, some plugin devs may be (= are) using VIPCS outside of the VIP CI context, for those, it maybe useful. Also, if you look at the rulesets, Minimum basically contains the majority of the "base" config, while Go builds on top of that and customizes.
If I remember correctly, a potential ruleset reorganisation is being discussedin #600. Probably best to keep the discussion in that issue. I've moved the ticket to the 4.x milestone now for higher visibility/findability. |
Even if they are, they should still be using the WordPress-VIP-Go. In this case, the changes for the variable analysis config is far more targeted to theme developers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
Good point. Either way, this change doesn't do any harm. |
While looking into #588, I noticed something which seemed odd, so I'm addressing this with this PR.
The configuration which was added in #690 (and addressed #588) was added to the
WordPress-VIP-Go
ruleset, not theWordPressVIPMinimum
ruleset from which theWordPress-VIP-Go
ruleset inherits.I can not think of any pertinent reason why the configuration should be added for one, but not the other and I couldn't find any discussion on this either, so I propose moving the configuration from the
WordPress-VIP-Go
ruleset to theWordPressVIPMinimum
ruleset.Ref: https://github.com/sirbrillig/phpcs-variable-analysis/releases/tag/v2.9.0