Skip to content

chore: raise Nextcloud floor to 32 (PHP 8.3) and drop the impossible stable31 CI leg - #154

Open
rubenvdlinde wants to merge 2 commits into
mainfrom
hotfix/nc-floor-32-main
Open

chore: raise Nextcloud floor to 32 (PHP 8.3) and drop the impossible stable31 CI leg#154
rubenvdlinde wants to merge 2 commits into
mainfrom
hotfix/nc-floor-32-main

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Why

Product directive from the PO (Ruben): the fleet standardises on Nextcloud 32 so it can require PHP 8.3 — "we want php 8.3 so going for min version nc 32 fleet wide is a good thing."

The governing rule (openconnector#1172 / #1173): an app's min-version must be >= the max of every <app> dependency's floor, or the App Store advertises a range the app cannot deliver — the dependency refuses to install and the app is non-functional there. openregister@development declares min-version 32 as of openregister#2384 (merged 2026-08-08T10:45Z); re-measured on this branch with an XML parser and confirmed.

Shape

Floor-only PR directly to main. Reason: main trails development by 188–5206 commits in every repo in this fleet, so a developmentmain merge would be a full release, not a floor fix.

Changes

1. appinfo/info.xml

field before after
nextcloud min-version 28 32
nextcloud max-version 34 34 (unchanged, deliberately not normalised)
php min-version 8.1 8.3

A prose comment was added above the nextcloud element. It contains no XML element syntax: floor guards here count raw regex matches of the nextcloud element across the whole file including comments, so a quoted example would read as a second, contradictory declaration and trip the guard. Validated before push: the file parses as XML and <nextcloud\b[^>]*> matches exactly once.

2. .github/workflows/code-quality.yml — drop the impossible CI leg

nextcloud-test-refs: '["stable31", "stable32", "stable33"]''["stable32", "stable33"]'

stable31 is removed because it tests an impossible configuration, not to trim coverage. A floor of 32 is enforced at install time, so occ app:enable refuses on NC31. The shared workflow runs app-enable as || echo "::warning::...", so the failure is only a warning and the job continues without its data layer, then dies ~70 seconds later on missing schemas — which reads like an app/migration fault and sends you to the wrong file entirely.

Order matters too: the newman / playwright / journeydoc jobs check out the server at fromJSON(inputs.nextcloud-test-refs)[0], so a stable31 sitting first put all of them on the one version the app cannot be enabled on.

This removes an impossible leg — it does not widen the matrix. No new refs are added. Some job names (PHPUnit (PHP 8.x, NC stable31)) will legitimately disappear from the check set.

Measurement notes

All version readings were taken with xml.etree.ElementTree, never grep.

  • Measured correction to the brief: this repo's main appinfo/info.xml contains exactly one <nextcloud> occurrence and no commented-out examples. The "literal examples inside comments" hazard cited in the rollout brief does not apply to this file — the single-match validation was still run and still passes.
  • openregister@main still declares min-version 28 (max 34). Only openregister@development is at 32. Stated explicitly so no reader infers a claim about main that is not true today.
  • This repo's main info.xml declares no <app> dependency at all (development declares openregister). So the dependency-max rule does not mechanically bind on main yet; the floor here is raised on the fleet directive and to match the development lineage. The install-time argument for dropping stable31 stands on the app's own floor of 32 regardless.
  • FYI: this repo's CI additional-apps installs ConductionNL/openregister at ref: main (floor 28), so openregister itself would install on NC31 — it is this app's new floor that makes the stable31 leg impossible.
  • Branch prefix: this PR was reopened from hotfix/nc-floor-32-main. The shared Branch Protection workflow requires PRs to main to originate from beta or hotfix/*; the original chore/* branch failed that gate legitimately, so the branch was renamed to comply rather than the gate overridden. Content is byte-identical.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ 7ad1598

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
composer ✅ 100/100
npm ✅ 389/389
PHPUnit
Newman
Playwright ⏭️
Hydra gates ⏭️

Quality workflow — 2026-08-08 16:09 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Status update — the floor has landed on development, which was the right route.

#148 merged 2026-08-09. Verified on development by parsing the real element with an XML parser (not a regex — <database min-version> is the classic trap, though this repo declares no <database> element):

development main
<nextcloud> min-version 32, max-version 34 min-version 28, max-version 34
<php> 8.3 8.1

raw '<nextcloud' occurrences including comments: 1 — no commented-out second declaration to misread.

The rule is satisfied on development: the only <app> dependency is openregister, whose development floor is 32, so 32 >= 32. max-version stays 34 — it matches openregister and corresponds to a real branch and CI leg (the repo tests stable32 and stable33). Deliberately not normalised upward to a version with no tag, no branch and no CI leg.

#148 also added scripts/check-nc-floor.js as a frontend-checks leg, and it is live and green in CIquality / Frontend Check (check:nc-floor) reports SUCCESS on subsequent PRs (#161, #163). Proven not to be a check that cannot fail: setting the element back to 28 locally makes it exit 1 with Nextcloud floor is 28, expected 32, and restoring it returns exit 0.

Leaving this PR open rather than closing it, because it is not fully superseded: main still declares 28 / PHP 8.1, so the App Store still advertises a range main cannot deliver. That is now a release-train question (developmentmain) rather than a floor question, and it is a decision for whoever owns the release, not something to slip in here.

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