Exclude test and tooling code from WPCS - #71
Merged
Merged
Conversation
phpcs.xml scans the whole repo and excluded only the plugin's own css and js. The integration test work added Playwright configs at the repo root, helper scripts under scripts/, and specs and fixtures under tests/, none of which are WordPress plugin code. WPCS lints them anyway and they fail on indentation and missing file docblocks. wpcs.yml runs on pull_request only, so nothing re-checked master after that merge and the breakage was invisible until the next PR opened against it. Every PR raised against master currently fails for this reason. Exclude tests/, playwright*.config.js, scripts/ and node_modules/. Only /siteimprove is copied into the published package, so the linted scope now matches the code that actually ships.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
phpcs.xmlscans the whole repo (<file>.</file>) and excluded only the plugin's own css and js. The integration test work merged around 11 September added Playwright configs at the repo root, helper scripts underscripts/, and specs and PHP fixtures undertests/. WPCS lints all of it as WordPress plugin code and it fails on indentation and missing file docblocks.wpcs.ymlrunson: pull_requestonly, so nothing re-checked master after that merge. The breakage stayed invisible until the next PR was opened against master, and every PR raised against master currently fails this way, including #70.Fix
Exclude
tests/,playwright*.config.js,scripts/andnode_modules/.Only
/siteimproveis copied into the published package (cp -r siteimprove/* deploy/in the deploy action), so the linted scope now matches the code that actually ships.Verification
Every file WPCS flagged is covered by the new patterns, and no plugin source is:
playwright*.config.js(3),scripts/*.js(2),tests/**(13, including the PHP fixtures and template)siteimprove/apart from the pre-existing css and js exclusionsI could not run phpcs locally, since PHP is not installed on this machine, so CI on this PR is the real check.
Note
If you would rather the PHP fixtures under
tests/stay linted, the narrower option is to exclude only the JS and keeptests/**/*.phpin scope. I excluded the whole directory because test fixtures are not shipped code.A more durable alternative to a growing exclusion list is changing
<file>.</file>to<file>siteimprove</file>, which scopes WPCS to the published directory permanently and means new root-level tooling can never break CI again. I did not do that here because it is a broader change than the one asked for, but it is worth considering as a follow-up.