Wpcs vip migration

[tomjn]

closes #1309

Removes the WordPress-VIP ruleset and associated sniffs in favour of moving it to an Automattic repository. This way all VIP rules are in the same location

An associated PR can be found at Automattic/VIP-Coding-Standards#161 that copies the existing sniffs to that repository

[jrfnl]

This will break way too much.

A number of the sniffs you are now removing have only just been deprecated for the 1.0.0 version. We will remove them in WPCS 2.0.0.

The same should be done for all the other sniffs.
So, please deprecate the sniffs instead of removing them.
Similarly, make the unit test files test the deprecation message. Do not blindly remove them.

[jrfnl]

I think removing this straight out will confuse a lot of people.

Please add something along the lines of the following:
Previously, WPCS also contained a WordPress-VIP ruleset. This ruleset has been removed in WPCS 1.0.0 in favour of the separately maintained [VIP Coding Standards](https://github.com/Automattic/VIP-Coding-Standards) library. If your code is intended to be run on the WordPress.com VIP platform, please use the rulesets provided by that library in addition to WPCS.

[jrfnl]

Removing the ruleset like this will break builds without explanation.

Please, clear out the ruleset instead (i.e. remove all the actual rules) and consider adding a bootstrap file with a deprecation message instead which exits with an error code so people will see it.

[tomjn]

Can you ellaborate on this? e.g. would a more verbose version of this suffice?

<?php
trigger_error('deprecated');

Or an echo? Or is there a PHPCS API needed?

[jrfnl]

Eh... this is an XML file ? Don't know why you'd think you could use PHP here.

Or is there a PHPCS API needed?

I have no clue what you mean by this.

Anyway, there are two approaches:

1. Clear out the ruleset + add a bootstrap file which will fail builds with a deprecation notice for the ruleset.

   In that case, just make it an empty ruleset:

   <?xml version="1.0"?>
   <ruleset name="WordPress VIP">
   	<description>The WordPress.com VIP Coding Standards have been moved.</description>
   </ruleset>

   You can even leave out the description, but I left it in as a form of documentation if people open it because they find it strange things aren't working anymore.

2. Leave the ruleset as it is (though silence the deprecation notices from sniffs). Add a bootstrap file with a deprecation notice which won't exit with a fail code.
   That way things will still work for people using the WordPress-VIP ruleset, but if they pay attention, they will get notified of the deprecation and can't be surprised by it anymore by the time we remove the ruleset completely.

[tomjn]

I mean in the bootstrap PHP file, I didn't realised you meant literally just an empty ruleset.xml with minimal tags

[jrfnl]

I mean in the bootstrap PHP file

Ah, well, there you would need to check what PHPCS has registered.
Something along the lines of:

• If the WordPress-VIP ruleset was the main ruleset being included, I'd suggest echo-ing out a deprecation message and doing and exit(1).
• If the WordPress-VIP ruleset was registered as part of a larger ruleset, you may just want to throw the deprecation message, but not throw an exit.
• If no reference to WordPress-VIP was found, do nothing and let PHPCS continue.

[jrfnl]

Removing the sniff like this will break any ruleset which explicitly includes it.

Please clear out the sniff instead and deprecated it.
See https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/VIP/SlowDBQuerySniff.php for an example.

[jrfnl]

Do not remove this file, but make it test the deprecation message instead. See the examples in other deprecated sniffs in the VIP directory.

[jrfnl]

Do not remove this file, but make it test the deprecation message instead. See the examples in other deprecated sniffs in the VIP directory.

[jrfnl]

👍

[tomjn]

I would note that anybody who works with VIP should know their way around PHPCS, and shouldn't be using the removed ruleset anyway.

I'm also mindful that once this and the other PR are merged, anybody using the VIP Coding standards will have 2 copies of this ruleset, each very different from each other. How does having 2 conflicting rulesets that clash present work? If the WPCS ruleset takes priority then the end result is that all VIP rulesets are broken regardless of location

[jrfnl]

I would note that anybody who works with VIP should know their way around PHPCS, and shouldn't be using the removed ruleset anyway.

Yes, except these rulesets are also used by a lot of plugin/theme developers who:

• either have chosen the wrong ruleset to use in the first place
• or have an ambition of getting their plugin/theme accepted for the VIP platform

Most of them are no PHPCS experts, nor should they need to be.

I'm also mindful that once this and the other PR are merged, anybody using the VIP Coding standards will have 2 copies of this ruleset, each very different from each other. How does having 2 conflicting rulesets that clash present work? If the WPCS ruleset takes priority then the end result is that all VIP rulesets are broken regardless of location

As long as the rulesets have distinct names, that should not be a problem. As I mentioned on Slack, don't call the new ruleset in the Automattic repo WordPress-VIP.

[tomjn]

I'm inclined to close this PR and redo this so that it can be done with deprecation in mind, and the need to rename the migrated ruleset at the VIP end makes life easier after thinking about it :)