refactor(settings): stop folding a fragment digest into the import version - #721
Conversation
…rsion Same reasoning as opencatalogi. OpenRegister gates with version_compare, which compares `+frag.<md5>` lexically rather than as semver build metadata, so the gate fired based on how two md5 hashes sorted — re-importing unchanged content about half the time and skipping real changes the other half. OpenRegister now hashes the merged configuration itself and skips on hash equality, detecting a changed fragment from the data. The version stays a version.
The comment block explaining why the fragment digest is no longer folded
into the version sat directly above `try {` with a blank line between,
which trips PHPCS's "no blank line following an inline comment" rule —
the one hard ERROR in the phpcs leg, and the only job failing on this PR
that is green on development.
Moved the explanation above the merge call it actually describes, and
stopped destructuring the hash at all: with the fold gone, $fragmentHash
was assigned and never read.
phpcs on lib/Service/SettingsService.php: 0 errors, 0 warnings.
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ |
Quality workflow — 2026-08-04 08:31 UTC
Download the full PDF report from the workflow artifacts.
…h gate My explanation added 13 lines to a 99-line method and pushed it to 105, over phpmd's 100-line threshold. The reasoning belongs in openregister#2325 where the measurements are; five lines and a pointer is enough here. AiService's TooManyPublicMethods on this run is pre-existing — this branch touches one file.
Resolved to the remote's content, which already dropped the fragment hash and the suffix. My local commit was trying to shorten a comment that the remote had already removed entirely, so the resolution adds nothing. Deliberately NOT shortening loadConfiguration further: phpmd's ExcessiveMethodLength on it, and TooManyPublicMethods on AiService, both already fail on procest's development branch. Adding lines to a method that is already over threshold would have made a pre-existing violation worse; fixing it belongs in its own change.
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ |
Quality workflow — 2026-08-04 08:56 UTC
Download the full PDF report from the workflow artifacts.
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-04 09:16 UTC
Download the full PDF report from the workflow artifacts.
|
Merging with
That one finding is being addressed separately by a scoped Why the change is worth havingIt removes the ADR-037 Same change as opencatalogi #798, merged earlier today. |
…figuration() (#725) phpmd on development flagged the last remaining finding: lib/Service/SettingsService.php:531 ExcessiveMethodLength The method loadConfiguration() has 102 lines of code. Current threshold is set to 100. It crossed the line in #721, which replaced the `+frag.<md5>` version suffix with a long explanatory comment. loadConfiguration() was doing two jobs: resolving the effective register configuration (read procest_register.json, parse it, deep-merge the ADR-037 register.d fragments) and importing it. Extract the first into a private readEffectiveConfiguration() returning ['data' => …] or ['error' => …], so loadConfiguration() stays a single import flow. Behaviour is unchanged — the same two failure shapes are returned to the caller in the same order. The extracted path had NO test: the only loadConfiguration test covers the OpenRegister-unavailable early return, which returns before any of this runs, so read/parse/merge could have broken silently. Add a test driving loadConfiguration() through to importFromApp, asserting it receives the on-disk version and parsed content, and that the version carries no `+` build suffix (the #721 invariant). Mutation-checked: returning an empty config array turns it red (0.13.2 -> 0.0.0). phpmd lib/ (597 files): exit 2 / 1 finding -> exit 0 / 0 findings. Positive-controlled both arms (injected probe yields 3 findings, exit 2). check:strict green: lint 0, phpcs 0, phpmd 0, psalm 0, phpstan 0, phpunit 0 (1691 -> 1692 tests, 5649 -> 5659 assertions). PHPUnit was run under Xdebug, because phpunit.xml sets beStrictAboutCoverageMetadata + failOnRisky and that check is INERT without a coverage driver — it silently caught nothing locally on #722 and only fired in CI. It flagged the missing @uses RegisterFragmentMerger here immediately.
1 participant
Same reasoning as opencatalogi. OpenRegister gates with
version_compare, which compares+frag.<md5>lexically rather than as semver build metadata, so the gate fired based on how two md5 hashes sorted — re-importing unchanged content about half the time and skipping real changes the other half.OpenRegister now hashes the merged configuration itself and skips on hash equality (openregister#2325), detecting a changed fragment from the data. The version stays a version.