Skip to content

chore: adopt Nextcloud's coding standard, .editorconfig and NC 34 - #189

Merged
rubenvdlinde merged 9 commits into
developmentfrom
chore/nextcloud-coding-standard
Aug 12, 2026
Merged

chore: adopt Nextcloud's coding standard, .editorconfig and NC 34#189
rubenvdlinde merged 9 commits into
developmentfrom
chore/nextcloud-coding-standard

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Part of the fleet-wide move to Nextcloud's coding standard. Recipe proven on nextcloud-app-template#142; enforced from ConductionNL/.github@main by 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: config the whole review
style: reformat 236 files, whitespace only — not worth reading line by line
chore: blame-ignore .git-blame-ignore-revs naming the reformat

What changed

  • .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 actually 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 tested version. Declared directly it was a dead dependency with no config file and no invocation anywhere.
  • phpcs.xml is a stub over the shared semantics-only ruleset; 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.
  • NC 34: 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::$server needed a hand-written sniff.
  • stylelint glob quoted, so stylelint expands it rather than the shell. Unquoted, 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.

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. 236 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.
e78eb70 touches 236 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.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ 8b1ce52

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer
npm ✅ 654/654
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 10:09 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.

Top-level blocks are now background-jobs, repair-steps, commands, settings,
navigations; <php> precedes <nextcloud>; and inside <repair-steps>,
<post-migration> precedes <install> (the schema's order is pre-migration,
post-migration, live-migration, install, uninstall - it does not reflect
execution order, which Nextcloud selects by event).

Ordering alone was not sufficient here. The <dependencies><app> entry is not
an ordering problem: the App Store schema has no <app> child under
<dependencies> at any position, so it can never validate. It was also inert -
OC\App\DependencyAnalyzer::analyze() handles only architecture, php, database,
command, lib, os and the server version, and never looks at "app" - so it
enforced nothing at install time. The dependency is now recorded as a comment
in the same place.

Verified: `xmllint --noout --schema info.xsd appinfo/info.xml` reports
"validates" (libxml2 2.12.10). The pre-change file failed the same command.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ 14f4d5e

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer ✅ 104/104
npm ✅ 654/654
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 10:43 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ c3e4414

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer ✅ 104/104
npm ✅ 654/654
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 11:19 UTC

Download the full PDF report from the workflow artifacts.

appinfo/info.xml declares <nextcloud min-version="32" max-version="34"/>, but
nextcloud-test-refs was '["stable34"]' — so the declared floor and the middle
major were advertised to the App Store with no job touching either.

This is the coding-standard migration's own defect: its rollout REPLACED the
ref list instead of extending it. The programme opened by reporting that
nothing was tested on NC 34 and, in fixing that, made 32 and 33 the untested
end. Same drift, other direction.

stable34 stays first because newman, playwright and journeydoc-capture all read
fromJSON(inputs.nextcloud-test-refs)[0] as their single server.

Verified green on all three refs against nextcloud/ocp ^34 on portaliq
(run 31599055849, six PHPUnit legs: 32/33/34 x PHP 8.3/8.4).
…onse

nextcloud/ocp v34's OCP\AppFramework\Http\DownloadResponse references
Symfony\Component\HttpFoundation\HeaderUtils, which was not in the
dependency tree. Three ExportsControllerTest download cases errored with
'Class "Symfony\Component\HttpFoundation\HeaderUtils" not found'.

Add symfony/http-foundation ^6.4 to require-dev so the class resolves
under static analysis and PHPUnit.
…ork service

The stub declared OCP\IInitialState, but OpenRegister's real
AppHost\Settings\GenericAdminSettings takes
OCP\AppFramework\Services\IInitialState. Application::register()
correctly passes the AppFramework service, so PHPStan reported a false
type mismatch at AppInfo/Application.php:322 against the inaccurate stub.

Align the stub with the class it stands in for.
The quoted stylelint glob widened coverage to .vue files that had never
been linted, surfacing 69 pre-existing rule-empty-line-before violations
across ten components. Whitespace only; no selector, declaration or
template changes.

Also quote the glob in the stylelint-fix script so --fix covers the same
file set the stylelint check does.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ 519b72b

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer ✅ 104/104
npm ✅ 654/654
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 18:31 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ e421dd2

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer ✅ 106/106
npm ✅ 654/654
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 19:07 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit b0cab96 into development Aug 12, 2026
68 of 74 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ e421dd2

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
test-l10n
check-gitignore
check-nc-floor
composer ✅ 106/106
npm ✅ 654/654
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-12 19:59 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde deleted the chore/nextcloud-coding-standard branch August 12, 2026 20:17
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