Skip to content

Commit 3e6a27b

Browse files
GaryJonesGaryJones
authored
Merge pull request #791 from Automattic/fix/contributing-doc
2 parents ca302d8 + 9146d1c commit 3e6a27b

File tree

1 file changed

+59
-39
lines changed

1 file changed

+59
-39
lines changed

‎.github/CONTRIBUTING.md

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ Since VIPCS employs many sniffs that are part of PHPCS, and makes use of WordPre
1717

1818
To determine where best to report the bug, use the first part of the sniff name:
1919

20-
Sniffname starts with | Report to
20+
Sniff name starts with | Report to
2121
--- | ---
2222
`Generic` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/)
2323
`PSR2` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/)
2424
`Squiz` | [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer/issues/)
25+
`Universal` | [PHPCSExtra](https://github.com/PHPCSStandards/PHPCSExtra/issues/)
2526
`VariableAnalysis` | [VariableAnalysis](https://github.com/sirbrillig/phpcs-variable-analysis/issues/)
2627
`WordPress` | [WordPressCS](https://github.com/WordPress/WordPress-Coding-Standards/issues/)
2728
`WordPressVIPMinimum` | [VIPCS](https://github.com/Automattic/VIP-Coding-Standards/issues/) (this repo)
@@ -112,47 +113,52 @@ The easiest way to do this is to add a `phpunit.xml` file to the root of your VI
112113
* To run the unit tests:
113114

114115
```sh
115-
phpunit --filter WordPressVIPMinimum $PHPCS_DIR/tests/AllTests.php
116+
composer test
116117
```
117118

118119
Expected output:
119120
```
120121
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.
121122

122-
.......................................... 42 / 42 (100%)
123+
........................................ 40 / 40 (100%)
123124

124-
43 sniff test files generated 117 unique error codes; 0 were fixable (0%)
125+
45 sniff test files generated 175 unique error codes; 0 were fixable (0%)
125126

126-
Time: 246 ms, Memory: 32.00 MB
127+
Time: 150 ms, Memory: 20.00 MB
128+
129+
OK (40 tests, 0 assertions)
127130
```
128131
129132
### Unit Testing conventions
130133
131-
If you look inside the `WordPressVIPMinimum/Tests` subdirectory, you'll see the structure mimics the `WordPressVIPMinimum/Sniffs` subdirectory structure. For example, the `WordPressVIPMinimum/Sniffs/VIP/WPQueryParams.php` sniff has its unit test class defined in `WordPressVIPMinimum/Tests/VIP/WPQueryParamsUnitTest.php` which checks the `WordPressVIPMinimum/Tests/VIP/WPQueryParamsUnitTest.inc` test case file. See the file naming convention?
134+
If you look inside the `WordPressVIPMinimum/Tests` subdirectory, you'll see the structure mimics the `WordPressVIPMinimum/Sniffs` subdirectory structure. For example, the `WordPressVIPMinimum/Sniffs/Performance/WPQueryParams.php` sniff has its unit test class defined in `WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.php` which checks the `WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.inc` test case file. See the file naming convention?
132135
133-
Lets take a look at what's inside `WPQueryParamsUnitTest.php`:
136+
Let's take a look at what's inside `WPQueryParamsUnitTest.php`:
134137
135138
```php
136139
...
137-
namespace WordPressVIPMinimum\Tests\VIP;
140+
namespace WordPressVIPMinimum\Tests\Performance;
138141
139142
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
140143
141144
/**
142145
* Unit test class for the WP_Query params sniff.
146+
*
147+
* @covers \WordPressVIPMinimum\Sniffs\Performance\WPQueryParamsSniff
143148
*/
144149
class WPQueryParamsUnitTest extends AbstractSniffUnitTest {
145150
146151
/**
147152
* Returns the lines where errors should occur.
148153
*
149-
* @return array <int line number> => <int number of errors>
154+
* @return array<int, int> Key is the line number, value is the number of expected errors.
150155
*/
151156
public function getErrorList() {
152-
return array(
157+
return [
153158
5 => 1,
154159
17 => 1,
155-
);
160+
31 => 1,
161+
];
156162
}
157163
...
158164
```
@@ -161,24 +167,35 @@ Also note the class name convention. The method `getErrorList()` MUST return an
161167
If you run:
162168

163169
```sh
164-
$ cd /path-to-cloned/phpcs
165-
$ ./bin/phpcs --standard=WordPressVIPMinimum -s --sniffs=WordPressVIPMinimum.VIP.WPQueryParams /path/to/WordPressVIPMinimum/Tests/VIP/WPQueryParamsUnitTest.inc
166-
...
167-
E 1 / 1 (100%)
168-
169-
170-
171-
FILE: /path/to/vipcs/WordPressVIPMinimum/Tests/VIP/WPQueryParamsUnitTest.inc
172-
--------------------------------------------------------------------------------------------------------------------------------
173-
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 4 LINES
174-
--------------------------------------------------------------------------------------------------------------------------------
175-
4 | WARNING | Using `post__not_in` should be done with caution. (WordPressVIPMinimum.VIP.WPQueryParams.post__not_in)
176-
5 | ERROR | Setting `suppress_filters` to `true` is probihited.
177-
| | (WordPressVIPMinimum.VIP.WPQueryParams.suppressFiltersTrue)
178-
11 | WARNING | Using `post__not_in` should be done with caution. (WordPressVIPMinimum.VIP.WPQueryParams.post__not_in)
179-
17 | ERROR | Setting `suppress_filters` to `true` is probihited.
180-
| | (WordPressVIPMinimum.VIP.WPQueryParams.suppressFiltersTrue)
181-
--------------------------------------------------------------------------------------------------------------------------------
170+
$ cd /path/to/vipcs
171+
$ ./vendor/bin/phpcs --standard=WordPressVIPMinimum -s --sniffs=WordPressVIPMinimum.Performance.WPQueryParams WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.inc
172+
173+
FILE: /path/to/vipcs/WordPressVIPMinimum/Tests/Performance/WPQueryParamsUnitTest.inc
174+
------------------------------------------------------------------------------------------------------------------------------------------------------
175+
FOUND 3 ERRORS AND 5 WARNINGS AFFECTING 8 LINES
176+
------------------------------------------------------------------------------------------------------------------------------------------------------
177+
4 | WARNING | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see
178+
| | https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.
179+
| | (WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in)
180+
5 | ERROR | Setting `suppress_filters` to `true` is prohibited.
181+
| | (WordPressVIPMinimum.Performance.WPQueryParams.SuppressFilters_suppress_filters)
182+
11 | WARNING | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see
183+
| | https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.
184+
| | (WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in)
185+
17 | ERROR | Setting `suppress_filters` to `true` is prohibited.
186+
| | (WordPressVIPMinimum.Performance.WPQueryParams.SuppressFilters_suppress_filters)
187+
21 | WARNING | Using exclusionary parameters, like exclude, in calls to get_posts() should be done with caution, see
188+
| | https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.
189+
| | (WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_exclude)
190+
29 | WARNING | Using exclusionary parameters, like exclude, in calls to get_posts() should be done with caution, see
191+
| | https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.
192+
| | (WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_exclude)
193+
30 | WARNING | Using exclusionary parameters, like exclude, in calls to get_posts() should be done with caution, see
194+
| | https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.
195+
| | (WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_exclude)
196+
31 | ERROR | Setting `suppress_filters` to `true` is prohibited.
197+
| | (WordPressVIPMinimum.Performance.WPQueryParams.SuppressFilters_suppress_filters)
198+
------------------------------------------------------------------------------------------------------------------------------------------------------
182199
....
183200
```
184201
You'll see the line number and number of ERRORs we need to return in the `getErrorList()` method.
@@ -189,23 +206,26 @@ The `--sniffs=...` directive limits the output to the sniff you are testing.
189206

190207
The ruleset tests, previously named here as _integration tests_, are our way of ensuring that _rulesets_ do check for the violations we expect them to.
191208

192-
An example where it might not would be when a ruleset references a local sniff or a sniff from upstream (WPCS or PHPCS), but that the violation code, sniff name or category name has changed. Without a ruleset test, this would go unnoticed.
209+
An example where it might not would be when a ruleset references a local sniff or a sniff from upstream (WordPressCS or PHPCS), but that the violation code, sniff name or category name has changed. Without a ruleset test, this would go unnoticed.
193210

194-
The `composer check` or `composer test-ruleset` commands run the `ruleset-test.php` files (one for each standard), which internally run `phpcs` against the "dirty" test files (`ruleset-test.inc`), and looks out for a known number of errors, warnings, and messages on each line. This is then compared against the expected errors, warnings and messages to see if there are any missing or unexpected violations or difference in messages.
211+
The `composer check` or `composer test-ruleset` commands run the `ruleset-test.php` files (one for each ruleset), which internally run `phpcs` against the "dirty" test files (`ruleset-test.inc`), and looks out for a known number of errors, warnings, and messages on each line. This is then compared against the expected errors, warnings, and messages to see if there are any missing or unexpected violations or difference in messages.
195212

196213
When adding or changing a sniff, the ruleset test files should be updated to match.
197214

198215
## Releases
199216

200-
- In a `changelog/x.y.z` branch off of `develop`, update the `CHANGELOG.md` with a list of all of the changes following the keepachangelog.com format. Include PR references and GitHub username props.
201-
- Create a PR of `develop` <-- `changelog/x.y.z`, but do not merge until ready to release.
202-
- Create a PR of `main` <-- `develop`, and copy-paste the [`release-template.md`](https://github.com/Automattic/VIP-Coding-Standards/blob/develop/.github/ISSUE_TEMPLATE/release-template.md) contents.
203-
- When ready to release, merge the change log PR into `develop`, then merge the `develop` into `main` PR.
217+
- Create a `release/x.y.z` branch off of `develop`.
218+
- In a `release/x.y.z-changelog` branch off of `release/x.y.z`, update the `CHANGELOG.md` with a list of all of the changes following the keepachangelog.com format. Include PR references and GitHub username props.
219+
- Create a PR of `release/x.y.z` <-- `release/x.y.z-changelog`, but do not merge until ready to release.
220+
- Create any other last-minute PRs as necessary, such as documentation updates, against the release branch.
221+
- When ready to release, merge the changelog and other branches into `release/x.y.z`.
222+
- Create a PR of `main` <-- `release/x.y.z`, and copy-paste the [`release-template.md`](https://github.com/Automattic/VIP-Coding-Standards/blob/develop/.github/ISSUE_TEMPLATE/release-template.md) contents.
223+
- When ready to release, merge `release/x.y.z` into `main`. Undelete the release branch after merging.
204224
- Tag the commit in `main` with the appropriate version number. Ideally, have it signed.
205-
- Close the current milestone.
206225
- Open a new milestone for the next release.
207226
- If any open PRs/issues which were milestoned for this release do not make it into the release, update their milestone.
208-
- Write a Lobby post to inform VIP customers about the release, including the date when the Review Bot will be updated (usually about 1.5 weeks after the VIPCS release).
227+
- Close the current milestone.
228+
- Create a PR of `develop` <-- `release/x.y.z` and merge in when ready.
229+
- Write a Lobby post to inform VIP customers about the release, including the date when the VIP Code Analysis Bot will be updated (usually about 2 weeks after the VIPCS release).
209230
- Write an internal P2 post.
210-
- Open a PR to update the [Review Bot dependencies](https://github.com/Automattic/vip-go-ci/blob/master/tools-init.sh).
211-
231+
- Open a PR to update the [VIP Code Analysis bot dependencies](https://github.com/Automattic/vip-go-ci/blob/master/tools-init.sh).

0 commit comments

Comments
 (0)