Skip to content

style(phpcs): clear the 10 phpcs ERRORS, leaving warnings untouched - #450

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/phpcs-error-debt
Aug 19, 2026
Merged

style(phpcs): clear the 10 phpcs ERRORS, leaving warnings untouched#450
rubenvdlinde merged 2 commits into
developmentfrom
fix/phpcs-error-debt

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Precondition for ConductionNL/.github#483, which makes phpcs errors fail the gate. That PR must not land while any repo still carries errors — .github@main is consumed live, so the flip reaches every repo the instant it merges.

All ten were formatting, not logic:

file n what
CommonCartridgeParser, MoodleBackupParser, AssessmentDrawResolver 8 @return docblock continuation lines padded to align under a very long type, reaching 159–202 chars against a 150 limit. The prose moves above the tag, reads the same, and fits.
LearningRecordShareVerifyController 2 $throttler and $logger added to the constructor without their @param lines

Measured, with a control — same tree, same instrument:

before: A TOTAL OF 10 ERRORS AND 110 WARNINGS WERE FOUND IN 99 FILES
after:  A TOTAL OF  0 ERRORS AND 110 WARNINGS WERE FOUND IN 97 FILES

The warning count is identical on purpose: this clears errors and deliberately does not touch the warning debt #483 leaves passing.

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.

All ten were formatting, not logic:

- CommonCartridgeParser / MoodleBackupParser / AssessmentDrawResolver (8): an
  `@return` docblock whose continuation lines were padded to align under a very
  long type, pushing them to 159-202 characters against a 150 limit. The prose
  moves above the tag, where it reads the same and fits.
- LearningRecordShareVerifyController (2): `$throttler` and `$logger` were added
  to the constructor without their `@param` lines.

Measured, with a control: on this tree phpcs reports
`A TOTAL OF 10 ERRORS AND 110 WARNINGS`; with the fix,
`A TOTAL OF 0 ERRORS AND 110 WARNINGS`. The warning count is identical on
purpose — this clears errors and deliberately does not touch the warning debt
#483 leaves passing.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/scholiq @ f1bed41

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
format
composer ✅ 104/104
npm ✅ 634/634
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-19 17:03 UTC

Download the full PDF report from the workflow artifacts.

gate-61 is diff-scoped on purpose — it "pulls the fleet along one listener at a
time" rather than letting a 149-registration backlog block unrelated PRs. This
PR's docblock reformat touched lib/Listener/AssessmentDrawResolver.php, which
pulled that registration into scope and surfaced pre-existing synchronous work.

Deferral is the gate's preferred answer and is the WRONG answer here. This
handler writes the AssessmentResult's `drawnItemRefs`, which is the set of items
the learner is about to be served, and the class contract is that it is "written
once; never recomputed by any later process". `TakeAssessmentView.vue` POSTs the
AssessmentResult and reads it straight back, so deferring to a background job
would serve an attempt whose drawnItemRefs is still the default `[]` for however
long the queue takes — an assessment with no questions. That is `correctness`,
one of the four closed ADR-078 categories, and the annotation carries the reason
rather than being a bare marker.

The work is also bounded: one ItemBank read plus a Fisher-Yates permutation over
drawCount items.

⚠️ My first placement of the tag REGRESSED phpcs — putting @listener-placement
above @PARAM tripped "Parameter tags must be defined first in a doc comment",
taking the file from 0 errors back to 1. Caught by re-running phpcs rather than
assuming the annotation was inert. Now: A TOTAL OF 0 ERRORS AND 110 WARNINGS,
the same warning count as before any of this.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/scholiq @ 3172ac3

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
format
composer ✅ 104/104
npm ✅ 634/634
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-19 17:50 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/scholiq @ 3172ac3

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
format
composer ✅ 104/104
npm ✅ 634/634
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-19 17:58 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 315b192 into development Aug 19, 2026
79 of 108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant