style(phpcs): clear the one phpcs ERROR (inline IF not allowed) - #2582
Merged
Conversation
added 2 commits
August 19, 2026 18:25
Precondition for ConductionNL/.github#483, which makes phpcs ERRORS fail the gate. That PR must not land while any repo still carries errors, because `.github@main` is consumed live and the flip reaches every repo the instant it merges. The ternary picking the postgres vs mysql quoting for `_uuid` becomes an explicit if/else. Same two branches, same values, no behaviour change — the sniff objects to the form, not the logic. Measured, with a control: before, `FOUND 1 ERROR AND 2 WARNINGS` naming line 614; after, `FOUND 0 ERRORS AND 2 WARNINGS`. The warning count is unchanged on purpose — this clears errors and leaves the warning debt #483 keeps passing.
The first attempt at this fix traded one gate for two others, and each caught
it in turn:
ternary -> phpcs: "Inline IF statements are not allowed"
if/else -> phpmd: "ElseExpression ... you can simplify the code"
if-then-override -> coverage ratchet: 9/344 -> 9/346 statements, "coverage of
the files this change touches dropped by 0.02%"
That last one is the ratchet working exactly as designed, on a change that adds
STATEMENTS without adding coverage — and it is right to, even though this is a
formatting-only edit. The lesson is to add no statements rather than to buy the
coverage back with a filler test.
`match` is one statement, precisely like the ternary it replaces, so the
statement count does not move. It is not an inline IF and it has no else.
Verified: `php -l` clean, no line over 150 characters, and all four
(needsPreUpdateState x isPostgres) combinations produce byte-identical output to
the original ternary.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 528/528 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-19 16:43 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 528/528 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-19 17:04 UTC
Download the full PDF report from the workflow artifacts.
…e count Third attempt, and the two failures in between were both real findings rather than noise. Four rules meet on this single line: ternary -> phpcs "Inline IF statements are not allowed" (the original) if/else -> phpmd "ElseExpression" if-then-override -> +2 statements, 344 -> 346, ratchet FAIL -0.02% match (2 arms) -> +3 statements, 344 -> 347, ratchet FAIL -0.03% I asserted in the last commit that "a match arm is one statement". That was wrong, and the ratchet said so: clover counts each executable LINE, so a two-arm match adds three. Any multi-line form adds uncovered statements to a change that alters no behaviour whatsoever. A single-expression lookup keyed on the boolean is one statement, exactly like the ternary it replaces, and is neither an inline IF nor an else. Verified: `php -l` clean; all four (needsPreUpdateState x isPostgres) combinations byte-identical to the original ternary; and the count of `existsColumns =` assignment lines is unchanged against HEAD at 2, which is what the ratchet actually measures.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 528/528 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-19 17:24 UTC
Download the full PDF report from the workflow artifacts.
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.
Precondition for ConductionNL/.github#483, which makes phpcs errors fail the gate. That PR must not land while any repo still carries errors —
.github@mainis consumed live, so the flip reaches every repo the instant it merges.The ternary picking postgres vs mysql quoting for
_uuidbecomes an explicit if/else. Same two branches, same values, no behaviour change — the sniff objects to the form, not the logic.Measured, with a control:
Warning count unchanged on purpose — this clears errors and leaves the warning debt #483 keeps passing.