chore: adopt Nextcloud's coding standard, .editorconfig and NC 34 - #2445
Open
rubenvdlinde wants to merge 8 commits into
Open
chore: adopt Nextcloud's coding standard, .editorconfig and NC 34#2445rubenvdlinde wants to merge 8 commits into
rubenvdlinde wants to merge 8 commits into
Conversation
Configuration only. The reformat is the next commit on purpose, so .git-blame-ignore-revs can name a revision containing nothing but whitespace. - .php-cs-fixer.dist.php + conduction/coding-standard, which extends nextcloud/coding-standard and can only ADD to it — enforced by that package's invariant test, not by review. - cs:check / cs:fix now run php-cs-fixer. They were aliases for phpcs/phpcbf, so the documented Nextcloud command reformatted code AWAY from Nextcloud's standard. - nextcloud/coding-standard dropped as a direct dependency. It arrives transitively at a version conduction/coding-standard has tested against; declared directly it was a dead dependency with no config and no invocation. - phpcs.xml is now a stub over the shared semantics-only ruleset, and the local phpcs-custom-sniffs/ copy is gone. The fleet was carrying six divergent versions of NamedParametersSniff.php — a custom RULE, not a setting. - .editorconfig, verbatim from nextcloud/server. No fleet app had one, so an editor configured by someone's previous Nextcloud work defaulted to tabs, which the old ruleset then rejected. - nextcloud/ocp -> ^34.0 and PHPUnit -> stable34. This app declared support for NC 34 while being analysed against 31, so a symbol REMOVED in 32/33/34 was invisible to the type checker. That is why the NC 34 removal of \OC::$server needed a hand-written PHPCS sniff. - the stylelint glob is quoted, so stylelint expands it rather than the shell. Unquoted, src/**/ matches exactly one directory level and nested components are silently unlinted. gate-65 (coding-standard-adoption) enforces all of the above from ConductionNL/.github@main. This app failed it; with this commit it passes.
Applied by php-cs-fixer with conduction/coding-standard. Tabs, same-line braces, (int)$x, single-space concatenation, ordered imports — Nextcloud's dialect, which this app now passes unchanged. 2574 file(s), no behaviour change. Isolated from the configuration change so .git-blame-ignore-revs can name a revision that touches nothing but formatting. Reviewing it line by line is not a useful activity; the previous commit is the review.
a34f528 touches 2574 files and changes no behaviour. Without this, every line it reflowed attributes to it and the real author is one --skip away. GitHub honours the file automatically; locally it needs `git config blame.ignoreRevsFile .git-blame-ignore-revs` once.
The previous commit changed composer.json without touching the lock, so
`composer install` refused with exit 4 and EVERY PHP job failed:
Required (in require-dev) package "conduction/coding-standard" is not
present in the lock file.
Required (in require-dev) package "conduction/hydra-gates" is not present
in the lock file.
Required (in require-dev) package "nextcloud/ocp" is in the lock file as
"v31.0.9" but that does not satisfy your constraint "^34.0".
Nothing was wrong with the reformat or the ruleset — the jobs never got as far
as running a tool. Measured on larpingapp#313 before this fix: phpcs, psalm,
phpstan and both PHPUnit legs red, all of them at `composer install`. Hydra
Gates passed in the same run, because it does not install composer
dependencies.
Now locked at conduction/coding-standard v1.0.0, conduction/hydra-gates v1.7.0,
nextcloud/ocp v34.0.2 — the last of which is the point of the exercise: this app
declares support for NC 34 and is now analysed against it.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ❌ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ❌ | ||||
| phpstan | ❌ | ||||
| phpmetrics | ❌ | ||||
| eslint | ✅ | ||||
| stylelint | ❌ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ❌ | ❌ | |||
| npm | ✅ | ✅ 546/546 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-12 10:10 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 | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 546/546 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-12 10:30 UTC
Download the full PDF report from the workflow artifacts.
The App Store's info.xsd declares <info> and its children as xs:sequence, so element ORDER is significant. This file was rejected by `xmllint --noout --schema info.xsd appinfo/info.xml`. Nextcloud's lint-info-xml workflow validates against exactly that schema, and ConductionNL/.github#383 adds the same check to the shared pipeline. Elements were moved into the schema's order. <version> and the <nextcloud> min/max-version declaration are unchanged. <documentation> moved ahead of <category>; top-level blocks are now background-jobs, repair-steps, commands, settings, activity, navigations, contactsmenu; and <activity>'s children are in settings, filters, providers order. One content fix was unavoidable. <notification><service> is not an ordering problem - the App Store schema has no <notification> element at any position, and Nextcloud's InfoParser never read one, so the declaration registered nothing. The notifier is registered in PHP, via registerNotifierService(AnnotationNotifier::class) in lib/AppInfo/Application.php, which is unaffected. Replaced by a comment pointing at that registration. Verified: `xmllint --noout --schema info.xsd appinfo/info.xml` reports "validates" (libxml2 2.12.10). The pre-change file failed the same command.
development landed the flow-run worker/guard work in eight files this branch had only reformatted. Resolved by taking development's content whole — the branch's sole change to those files was commit a34f528 (whitespace) — then re-running php-cs-fixer over the merged tree. Taking the branch's side would have discarded ~660 lines of real development work; taking development's and reformatting loses nothing, because formatting is reproducible and the logic is not. php-cs-fixer reformatted 12 files: the 8 conflicted ones plus the 4 development added while this branch was open (FlowRunGuard, FlowRunExpired, FlowRunGuardTest, ConfidentialityClauseEnforcementTest). composer.lock did not conflict — development never touched it. nextcloud/ocp stays at v34.0.2. cs:check: Found 0 of 2578 files that can be fixed.
Three separate causes, all of them this branch's doing, none of them a
defect in the code being reformatted.
phpcs (26 errors -> 0)
The stub ruleset enforces a 150-character line limit. php-cs-fixer's
phpdoc_align aligns a wrapped `@return` continuation to the column
after the type on the FIRST line, so a long array shape pushes its own
description past 150 — the formatter mechanically creates the
violation the linter then reports. Measured: `git show
origin/development:<file>` has ZERO over-long lines in every file
flagged here. The fix is to stop hanging prose off a long `@return`
and put it in the description, where it reads better anyway.
AggregationRunner keeps its wrapped union and gains an `AggValues`
alias instead. Joining that union onto one line makes PHPStan resolve
it properly for the first time, which surfaces unrelated pre-existing
type debt two thousand lines away — not this commit's business.
SettingsService::convertToBytes() loses its switch fall-through.
php-cs-fixer's no_break_comment requires the marker to read exactly
`no break`; PHPCS's Squiz.Commenting.InlineComment.NotCapital requires
a capital. No spelling satisfies both, so the fall-through goes and
each case states its own multiplier. Same arithmetic.
psalm (5 errors -> 0), baseline regenerated
Two of the three drifted entries drifted for a reason worth naming:
psalm baselines match on a CODE SNIPPET, and the reformat moved the
opening brace onto the declaration line, so
`public function __construct(Entity $entity, string $systemSlug)`
stopped matching `... $systemSlug) {`. Any app in this fleet with a
snippet-matched psalm baseline will drift the same way on reformat.
The third, allowEvalScript, is an NC 34 removal.
phpstan (21 errors -> 0), baselined in phpstan-baseline-nc34-ocp.neon
All 21 come from analysing against ocp 34 instead of 31. Pre-existing
code; the analyser just got the API the app claims to support. Four
are real NC 34 removals that will fatal at runtime. Baselined rather
than fixed because porting to NC 34 is not a reformat, and because
several of the rest are constant-false comparisons where the
always-false predicate may be the only thing suppressing a wrong code
path. Every entry is diagnosed in openregister#2450.
phpmd was already clean on the merged tree; no change needed.
… glob exposes development landed the same coding-standard migration in parallel (b22f86d), so this merge is mostly the two sides agreeing. Config conflicts, resolved deliberately rather than by side: .editorconfig ours — a superset; development's copy omits the package*.json, psalm-baseline and config/*config.php sections. .php-cs-fixer.dist.php ours — `__DIR__ . '/...'`. The standard this file configures mandates single-space concatenation, and the file is outside the finder's paths so nothing would fix it. phpcs.xml theirs. composer.json theirs (tabs, per the .editorconfig both sides added) with nextcloud/ocp back to ^34.0 — the one thing this branch is for. development kept ^31.0. package.json theirs. Both sides quoted the stylelint glob; theirs is three quoted globs, ours was one brace expansion. Same widening. composer.lock theirs, then regenerated. Read back: nextcloud/ocp v34.0.2, conduction/hydra-gates v1.7.1, conduction/coding-standard v1.0.0. stylelint: 142 errors -> 0 The quoted glob is the cause and that is the point of it. Unquoted, `src/**/*.vue` is expanded by bash without globstar, so `**` collapses to one directory level and every nested component went unlinted. These 142 are not regressions; they are the backlog the old glob was hiding. 132 were auto-fixable (rule-empty-line-before). The other 10 were hand-fixed and each one is a copy-paste artefact, not a style opinion: WebhooksIndex.vue three rule blocks duplicated verbatim ~100 lines below the originals; deleted. WebhookLogsIndex.vue same, two blocks; deleted. SearchTrailIndex.vue .searchTermColumn declared twice with disjoint properties; merged into the first. ViewObject.vue :deep(.cm-editor) likewise; merged. MergeObject.vue .object-id likewise; merged. UploadFiles.vue `.success` declared twice with CONFLICTING colours — `green` then `var(--color-success)`. The later one already won, so the first was dead; deleted it rather than the live one. Also `calc(var(--min-size)* 2)` -> spaced. Every merge kept the full declaration set, and no duplicate sat behind a competing rule of equal specificity, so the computed styles are unchanged. Verified on the merged tree: cs:check 0 of 2578, phpcs 0, phpstan 0, psalm 0, phpmd 0, eslint 0 errors, stylelint 0.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 546/546 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-12 13:35 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.
Part of the fleet-wide move to Nextcloud's coding standard. Recipe proven on nextcloud-app-template#142; enforced from
ConductionNL/.github@mainby gate-65 coding-standard-adoption, which this app fails today and passes with this PR.Policy: Conduction code must pass Nextcloud's own checks unchanged. We may be stricter, never different.
Three commits, deliberately separated
chore:configstyle:reformatchore:blame-ignore.git-blame-ignore-revsnaming the reformatWhat changed
.php-cs-fixer.dist.php+conduction/coding-standard, which extendsnextcloud/coding-standardand can only add to it — enforced by that package's invariant test, not by review.cs:check/cs:fixnow actually run php-cs-fixer. They were aliases forphpcs/phpcbf, so the documented Nextcloud command reformatted code away from Nextcloud's standard.nextcloud/coding-standarddropped as a direct dependency — it arrives transitively at a tested version. Declared directly it was a dead dependency with no config file and no invocation anywhere.phpcs.xmlis a stub over the shared semantics-only ruleset; the localphpcs-custom-sniffs/copy is gone. The fleet was carrying six divergent versions ofNamedParametersSniff.php— a custom rule, not a setting..editorconfig, verbatim fromnextcloud/server. No fleet app had one.nextcloud/ocp→^34.0, PHPUnit →stable34. This app declared support for NC 34 while being analysed against 31, so a symbol removed in 32/33/34 was invisible to the type checker — which is why the NC 34 removal of\OC::$serverneeded a hand-written sniff.src/**/matches exactly one directory level and nested components go unlinted.Verification
Reformatted with a shared php-cs-fixer across all 18 apps: 9,656 files, 0 remaining findings anywhere. Every changed PHP file re-linted with
php -l.Expect red
This app's base branch already fails several quality jobs, and moving to NC 34 will surface more. That is intended — the point is to align with Nextcloud, not to keep the board green.