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

Add note regarding WordPress-VIP ruleset #1403

Closed
wants to merge 2 commits into from

Conversation

GaryJones
Copy link
Member

Quick and temporary workaround to reduce the chance of new folks using it:

I honestly believe this ruleset is damaging, and the longer it remains the higher the chances of new people finding it and using it.

See #1309.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

  • It was originally for [WordPress.com VIP coding requirements] - is this sentence missing the word intended or is it just me ?
  • I'm a bit wary about using the word soon in this context all things considering.

@GaryJones
Copy link
Member Author

It was originally for [WordPress.com VIP coding requirements] - is this sentence missing the word intended or is it just me ?

Added, and clarified further.

I'm a bit wary about using the word soon in this context all things considering.

Changed to "in due course".

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

  • this is no longer used or recommended by the WordPress.com VIP team or their clients. - Do you mean for their clients ?

I'm wondering if we shouldn't also remove the VIP ruleset and the VIP sniffs - <exclude name="WordPress.VIP" /> - from the WordPress ruleset in this PR.
That way the WPCS side of things would be completely covered by this PR.

Opinions ?

@GaryJones
Copy link
Member Author

This was meant as a quick workaround whilst the finer points of how such a removal would be done. It was building on Tom's information that the VIP team no longer use it, or recommend it to their clients.

I think making an actual change to the characteristics of WPCS, like removing the ruleset, should be its own PR, even if that includes a further change to this readme about its inclusion.

@JDGrimes
Copy link
Contributor

Just to make sure that we are all on the same page, @jrfnl was suggesting that the WordPress.VIP sniffs be excluded from the WordPress ruleset, so that the VIP sniffs would no longer be used by WPCS by default—only if they were explicitly included via a custom ruleset.

I think that makes sense, as otherwise, I'd think a lot of folks would still end up using the VIP sniffs via the WordPress ruleset. It doesn't make sense to tell people not to use it, and then offer them a ruleset that includes it, IMO.

@jrfnl jrfnl added this to the 1.0.0 milestone Jul 2, 2018
@tomjn
Copy link
Contributor

tomjn commented Jul 3, 2018

I can say VIP doesn't recommend the ruleset fullstop, client or not client

@GaryJones
Copy link
Member Author

@GaryJones GaryJones self-assigned this Jul 16, 2018
@jrfnl
Copy link
Member

jrfnl commented Jul 16, 2018

Closing as @GaryJones' textual improvements were implemented in #1410.

@jrfnl jrfnl closed this Jul 16, 2018
@GaryJones GaryJones deleted the gj/1309-vip-ruleset-note branch July 16, 2018 12:31
@GaryJones GaryJones removed this from the 1.0.0 milestone Aug 22, 2018
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.

4 participants