Use US spellings throughout the codebase - #1457
Merged
Merged
Conversation
Tom asked for US spellings; the tree carried a mix, most visibly "enrolment" beside "enrollment" in the Secure Boot work that just landed. Prose only -- comments, docblocks, test descriptions, ADRs, help text and the thirteen user-facing gettext strings that carried a UK form. The rewrite works from an explicit word list rather than a blanket -ise -> -ize, which would have corrupted advertise, exercise, surprise and a dozen others that are -ise in both dialects, and it skips every $-prefixed token so no identifier moves. Five things are deliberately left alone, each because a machine reads them and a rename would break something: - `colour --rgb` and `cpair --` are iPXE's OWN command names. There is no `color` command; rewriting these silently breaks every boot menu's appearance. So is the `(colour)` parenthetical in the FOG_IPXE_* help text, which exists precisely to tell an admin what to type. - `FOG_IPXE_MAIN_COLOURS` and its siblings are globalSettings ROW NAMES, inserted by schema.php. Renaming them orphans the setting on every existing install. - The taskStates seed row `(5,'Cancelled',...)` is a database value, not source text. Changing it here would change fresh installs only, so an upgraded server would read "Cancelled" and a new one "Canceled" -- and the label is translated by its own text, so the new spelling would silently lose every locale's string. Converging them needs a schema step, which is a migration rather than a spelling fix. - `value="cancelled"` on the task list's Recent pane is the sender and `case 'cancelled':` in the same file is the receiver. Rewriting one without the other empties the pane; both stay as they are. - `getCancelledState()` is public and the `CANCELLED_STATE` hook is part of the plugin ABI. ADR filenames keep their UK spellings -- other files, and one file in the fos repo, reference them by name, so renaming them is its own change. tests/run-all.sh: 185 passed, 0 failed. Both phpstan passes clean. Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 29, 2026
osiktech
pushed a commit
to osiktech/fogproject
that referenced
this pull request
Sep 9, 2026
FOGProjectGH-1457 made the tree clean; nothing stopped it drifting back. This is the gate. A whole-tree scan, not a diff. The obvious shape is "check only the lines this pull request adds", and it needs a base ref -- which this file cannot see or set, because the job that runs it lives in FOGProject/fog-workflows. A shallow clone has no merge-base, so the check would find nothing to look at and pass, for a reason that has nothing to do with spelling, silently and on every pull request. A gate that can only report success is worse than no gate, because it also reports "verified". Scanning everything needs no ref, cannot skip, and is only possible because the tree is already clean -- the same principle the phpstan job is built on: a gate that can only be satisfied, never one that grows. Two patterns, because one right-hand boundary cannot serve both shapes. The word/camelCase one refuses a following lower-case letter, so `enrol` does not fire inside `enrolled` or `enrollment`. The ALL CAPS one refuses a following letter of any case, or `ENROL` matches inside `ENROLL_SECUREBOOT` and the gate fails on a correctly spelled task type. Both are case-sensitive: a trailing /i also folds the [a-z] and [A-Z] in the boundary assertions, which collapses the camelCase hump into "letter, letter" -- no boundary at all. Files come from `git ls-files --cached --others --exclude-standard`. A filesystem walk drags in packages/web/lib/plugins/, which is gitignored, root-owned and written by the installer rather than by this repository; plain `git ls-files` is blind to a file a developer has written but not staged, which is how a new class passed psr4-layout locally and failed in CI. That pair is exactly right: tracked, plus new, minus ignored. Writing it found what the sweep had missed, in three classes: - ALL-CAPS and capitalised forms (ENROLMENT, RECOGNISES, Normalising, Initialised, Behavioural) that an enumerated word list never listed. The sweep now carries the match's own case instead. - `relabelling`, and the words afterwards/towards/judgement/ageing/ artefact/programme. - UK spellings inside identifiers the $-variable guard had skipped: $cancelled, $cancelledIDs, $jobcancelled, $SessCancelled, $inTaskCancelledIDs. All file-local, all renamed. Three identifiers stay UK and are allowlisted with the reason: getCancelledState(), the CANCELLED_STATE hook, and the 'cancelledState' key that hook hands a plugin in its arguments array. All three are the plugin ABI. Proven by mutation, not by being green: a lower-case word, a Title-case word, an ALL-CAPS word, an ALL-CAPS word inside SNAKE_CASE, a camelCase identifier, a UK word in an untracked new file, and a broken file enumeration each make it fail; `enrollment`, `enrolled`, `enrolling` and `ENROLL_SECUREBOOT` each leave it green. tests/run-all.sh: 186 passed, 0 failed. Both phpstan passes clean. Co-Authored-By: Claude <noreply@anthropic.com>
osiktech
pushed a commit
to osiktech/fogproject
that referenced
this pull request
Sep 9, 2026
…2-3)
Counting an imaging run out of taskLog takes three rules, none obvious from
the schema and every one of them silent when broken -- the query still
returns a plausible number and nothing distinguishes it from a right one by
eye. Since ADR 0022 decision 3 retired imagingLog and made taskLog the
imaging record, those rules have lived as a comment inside
DashboardPage::get30day(), which made that method their definition as well
as their only caller. The second consumer would have got them wrong.
fold to one row per task a task through three states is one image
exclude the canceled state TaskLog::recordState() writes logImageName
on every transition including cancellation,
so a deploy queued and canceled without ever
starting carries an image name and touched
nothing. One canceled MID image has its
In-Progress row and did, so the exclusion is
on the ROW and not on the task
attribute to MIN(createTime) a run spanning midnight writes rows on two
days and is one run, on the first
src/Audit/ImagingStats.php is the one place they live now, shaped after
ActivityWindow: read only, no write side, bounded, its SQL reachable for a
test the way TaskManagement::_logQueryFrom() is. get30day() is a caller.
Zero filling is part of the contract rather than a courtesy to that caller.
A day with no runs produces no row, and every chart library draws a line
straight across a missing day -- so an idle week reads as steady activity.
MAX_DAYS is 366 rather than 365 because the dashboard's widest view is a
year and a leap year is 366 days; a cap of 365 would clip the widest shipped
view and be reported as "the 1 Year graph is missing a day".
The bounds must be on FOG's clock. get30day() builds them with niceDate()
and converts the series back the same way -- strtotime() on a bare date
resolves in PHP's timezone while every bound is on FOG's, which would plot
the points off by the difference between the two clocks. That is the exact
failure the rollup exists to prevent, one line after calling it.
tests/imaging-stats.test.php, 40 checks, replaces
tests/imaging-count-excludes-cancels.test.php. That file asserted on the
query while it lived in get30day(); the query moved, so its five checks this
one lacked moved with it rather than being dropped. Three halves that fail
differently: the SQL semantics run for real against TEMPORARY tables because
a GROUP BY is not provable by reading it, the series contract runs through a
fake connection with canned rows, and the page is asserted to no longer
CONTAIN the query -- checking only that it calls ImagingStats would pass with
the old query still sitting beside the call.
Every check was mutation verified against a live database: each of the three
rules removed or inverted in turn, the image-name filter removed, the zero
fill removed, the DatePeriod bound left exclusive, the cap and the reversal
removed, MAX_DAYS lowered to 365, and the query re-inlined into the page.
All eleven turn the check that names them red, at exit 1.
One check inherited from the old test was a FAKE GATE and is replaced rather
than carried over. `/SELECT\s+`taskID`.*?BETWEEN\s+:start\s+AND\s+:end/s`
was meant to pin that the window bounds the INNER scan; under /s the lazy
wildcard reaches past the derived table's closing paren, so moving the bound
to the outer query -- which makes every poll scan all of taskLog to compute a
MIN it discards -- left it green. Asserted by position now, plus that the
bound is on the indexed column rather than on the MIN() alias, and both forms
were run against that exact mutation.
The DB half needs no CREATE DATABASE right and skips without FOG_TEST_DSN,
the convention schema-executes and tasklog-report-retention already use.
Two things found while in the file and fixed. Three of the five day-range
links in the imaging card shared id="graph-day-filters-90"; the JS keys off
.graph-days and rel so nothing was broken, but duplicate ids are invalid and
the next thing to reach for one by id gets whichever came first. And the new
class needed registering in bin/psr4-scan.php's TABLE -- that gate reads
git ls-files, so an unregistered new class passes locally and fails in CI.
Spelling follows the sweep in FOGProject#1457: canceled in prose and in the bind
placeholder, with getCancelledState() left alone as an existing identifier.
Co-Authored-By: Claude <noreply@anthropic.com>
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.
The tree carried a mix of UK and US spellings — most visibly
enrolmentbeside
enrollmentin the Secure Boot ledger work that just landed, butalso
behaviour,normalise,recognise,neighbour,cancelled,colour,analyse,licenceandcentrescattered through comments,docblocks, test descriptions and ADRs.
Prose only. The rewrite works from an explicit word list rather than a
blanket
-ise→-ize, which would have corruptedadvertise,exercise,surpriseand a dozen others that are-isein bothdialects, and it skips every
$-prefixed token so no identifier moves.Thirteen user-facing
_()strings change (Selected tasks cancelled!→canceled!,Automatic enrolment→enrollment, and so on). Thepre-commit hook regenerated
messages.potand msgmerged the.pofilesaccordingly, so those thirteen fall back to English until a translator
picks them up.
What is deliberately left alone
Each of these is read by a machine, so a rename would break something:
colour --rgb,cpair --colorcommand — rewriting these silently breaks every boot menu's appearance.(colour)in theFOG_IPXE_*help textFOG_IPXE_MAIN_COLOURSand siblingsglobalSettingsrow names inserted byschema.php. Renaming orphans the setting on every existing install.taskStatesseed(5,'Cancelled',…)value="cancelled"/case 'cancelled':getCancelledState(),CANCELLED_STATEfos. Renaming them is its own change.The first three were found by reading the diff, not by guessing — the
first cut of this sweep did rewrite
colour --rgb 0x00567a 1 ||in theFOG_IPXE_MAIN_COLOURSdefault.Verification
tests/run-all.sh— 185 passed, 0 failedvendor/bin/phpstan analyse— no errorsvendor/bin/phpstan analyse -c phpstan-tests.neon— no errorsDownstream
None. No route class,
$databaseFieldskey, API field or REST path changes.The matching prose sweep for FOS is FOGProject/fos#168.