chore(config): move AppStoreSource off the deprecated IConfig app-value API - #343
Merged
Merged
Conversation
…ue API Completes the migration started for ForgeRegistry. OCP\IConfig::getAppValue and setAppValue are @deprecated 29.0.0 in favour of IAppConfig. 92 -> 87, and versioniq now has no deprecated app-value call left. This one could not be a straight swap, which is why it was held back from the ForgeRegistry PR: AppStoreSource also calls getSystemValueString, which exists ONLY on IConfig. So it takes BOTH now, IConfig purely for the system value and IAppConfig for the five app values.⚠️ The new parameter goes AFTER the existing IConfig one, and all five test constructions are POSITIONAL, so every one of them is updated in the same commit. Adding a parameter mid-signature without walking the call sites is how a green suite turns into `Argument #N must be of type X` at runtime. The mock stubs move with the calls: getAppValue -> getValueString and setAppValue -> setValueString. setValueString returns bool where setAppValue returned void, so those callbacks return true rather than nothing; a void callback would have handed PHPUnit a null for a bool return. Verified with the repo own vendor/bin/psalm 5.26.1 on PHP 8.3: 92 -> 87 with the baseline emptied, regenerated baseline is green, and the five stale DeprecatedMethod entries are gone rather than merely unused. PHPUnit was NOT run here, it needs the Nextcloud server bootstrap.
Contributor
Quality Report — ConductionNL/versioniq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 29/29 | |||
| npm | ✅ | ✅ 302/302 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 18:59 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
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.
Completes what the
ForgeRegistryPR started.OCP\IConfig::getAppValueandsetAppValueare@deprecated 29.0.0in favour ofIAppConfig. 92 -> 87, and versioniq now has no deprecated app-value call left anywhere.This one could not be a straight swap, which is exactly why it was held back:
AppStoreSourcealso callsgetSystemValueString, which exists only onIConfig. So it now takes both —IConfigpurely for the system value,IAppConfigfor the five app values.The part worth reviewing
The new parameter goes after the existing
IConfigone, and all five test constructions are positional, so every one is updated in the same commit. Adding a parameter mid-signature without walking the call sites is how a green suite turns intoArgument #N must be of type Xat runtime.The stubs move with the calls:
getAppValue->getValueString,setAppValue->setValueString.setValueStringreturnsboolwheresetAppValuereturnedvoid, so those callbacks nowreturn true— a void callback would have handed PHPUnit anullfor aboolreturn type.Verification
vendor/bin/psalm5.26.1 on PHP 8.3: 92 -> 87 with the baseline emptied, regenerated baseline green, and the five staleDeprecatedMethodentries are gone rather than merely unused. PHPUnit not run locally (needs the server bootstrap).