Repository navigation
Fix two latent VIPCS phpcs violations blocking CI - #1037
Merged
Merged
Conversation
A newer automattic/vipwpcs 3.x release, floated in because the repo has no committed composer.lock, started flagging two existing lines: the WP-CLI status-migration query's posts_per_page => -1, and the wp_redirect test callback that throws to unwind the handler. Both are latent on develop but hidden by the phpcs result cache, so they only surface on a cold-cache CI run and block otherwise-clean pull requests. Add a justified ignore for NoPaging on the CLI query, where fetching every matching ID in one pass is correct for a one-off migration, and extend the test's existing AlwaysReturnInFilter ignore to cover the new TerminatingInsteadOfReturn sub-sniff. Both suppressions document why the pattern is intentional rather than weakening the ruleset.
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
1 participant
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.
Summary
CI recently began failing the PHP coding-standards job on branches that had not touched the offending code. The cause is a combination of two things. The repository has no committed
composer.lock, so each CI run installs the latestautomattic/vipwpcsmatching^3.0; a newer 3.x release tightened two sniffs. The php-lint workflow also caches phpcs results with broad restore keys, sodevelop's runs keep restoring a stale cache that marks the affected files clean and never re-evaluates them under the newer ruleset. The violations are therefore latent ondevelopand only surface when a pull request happens to run against a cold cache — at which point an unrelated PR is blocked by a failure it did not introduce.Two lines are flagged. The WP-CLI status-migration command in
class-cli.phpqueries withposts_per_page => -1, which theNoPagingsniff prohibits in a VIP context; here it is correct, because a one-off CLI migration should fetch every matching ID in a single pass and already limits itself to'fields' => 'ids'. Separately, a test callback on thewp_redirectfilter throws to unwind the handler before it reachesexit, which the newerAlwaysReturnInFiltersniff now flags under a newTerminatingInsteadOfReturnsub-code that the existing ignore did not cover.Rather than weaken the ruleset, both are addressed with justified, narrowly-scoped
phpcs:ignorecomments that document why the pattern is intentional. The existing ignore on the test is extended to list both sub-sniffs.With these in place, a cold
composer csrun is clean across the whole repository, so CI passes on any branch regardless of cache state.Test plan
rm -f .phpcs-cache && composer csexits 0 across the whole repository