chore: untrack the built engine (CHORE-UNTRACK-BUILT-ENGINE) - #110
Conversation
…-ENGINE) CTLD.lua is generated and committed anyway; .gitignore calls it deliberate, for a bootstrap-era reason that no longer holds. Nothing points at the repository file, and VMCT — the consumer everyone assumed depended on it — takes the release asset instead (vendored.yaml pins 2.0.0-rc3 and watches github-release). The cost is paid on every code change: 26 of the 28 merges touching src/ over 30 days carried the regenerated file. The lot leads with the trap rather than the deletion. python-quality runs on ubuntu and never builds the engine, so removing the file alone takes the suite from 262 passed to 234 passed / 27 skipped / 1 failed with CI still green — measured, not assumed. Build first, delete second. The failure is a separate defect the measurement exposed: test_inject_into_miz crashes where its neighbours skip. Depends on FEAT-DEV-BUILD-CHANNEL: a release and the dev pre-release both attach CTLD.lua, which is what makes the removal safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…UNTRACK-BUILT-ENGINE 01) python-quality runs on ubuntu and never built CTLD.lua: 27 tests guarded by skipif(not CTLD.lua) only ran because the artifact is committed. Measured by removing the file — 262 passed becomes 234 passed, 27 skipped, 1 failed, with CI still green. The build has to come first, so that untracking the artifact later cannot silently drop a whole suite. merge_CTLD.ps1 stays the single build path; it now runs on Linux too. Three literal Windows paths were in the way, not the two the ticket listed: generate_i18n_dicts.ps1 carries the same "..\.." and is called by the merge script. All three are composed one segment at a time, which Join-Path resolves per platform. python-quality's paths: filter gains src/** and tools/build/** — the job now builds the engine, so a change to either can break it. Local Windows build unchanged: same command, and the rebuilt CTLD.lua differs by no line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CTLD.lua is generated by merge_CTLD.ps1 and was committed anyway, on a bootstrap-era ground — "available at repo root for DCS missions" — that no longer holds: nothing points at the repository file, and VMCT, the consumer everyone assumed depended on it, takes the release asset (vendored.yaml: "re-download the CTLD.lua asset from the matching release"). Meanwhile 26 of the 28 merges touching src/ over 30 days carried the regenerated file: a one-megabyte generated diff in almost every code PR, and a guaranteed conflict between parallel branches. Untracking is safe now because FEAT-DEV-BUILD-CHANNEL publishes the engine where people take it: attached to each release, and to the floating dev pre-release rebuilt on every merge. Verified on the first dev build — CTLD.lua attached, declaring 2.0.0-rc6-182ec25. test_inject_into_miz gains the guard its neighbours have. It used to fail with KeyError: 'injected' when no engine was built, naming neither the engine nor the build; without CTLD.lua the suite now reports 234 passed / 28 skipped / 0 failed, and 262 passed with it. No history rewriting: the 471 past blobs weigh 2.8 MiB packed. Docs updated where the assumption lived: CLAUDE.md now says never commit as well as never hand-edit, and building-and-testing (EN + FR) says where to get an engine per role. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reviewer's GuideThis PR stops committing the generated CTLD.lua engine by making the PowerShell build scripts portable to Linux, ensuring CI builds the engine before tests, adding a missing skip guard in a web-app test, updating documentation to reflect that CTLD.lua is git-ignored and obtained from releases/dev builds, and finally untracking/ignoring CTLD.lua in git. Sequence diagram for python-quality CI building CTLD.lua before testssequenceDiagram
participant GitHubActions as python-quality_job
participant MergeScript as merge_CTLD.ps1
participant I18nScript as generate_i18n_dicts.ps1
participant Pytest as pytest
GitHubActions->>MergeScript: run ./tools/build/merge_CTLD.ps1
MergeScript->>MergeScript: Resolve-Path scriptDir
MergeScript->>MergeScript: Resolve-Path repoRoot (Join-Path ".." "..")
MergeScript->>MergeScript: Join-Path repoRoot src
MergeScript->>MergeScript: Join-Path repoRoot CTLD.lua
MergeScript->>MergeScript: Join-Path (Join-Path repoRoot tools) ctld-tools
MergeScript->>I18nScript: generate_i18n_dicts.ps1 -Apply
I18nScript->>I18nScript: Resolve-Path repoRoot (Join-Path ".." "..")
I18nScript-->>MergeScript: i18n dicts synced
MergeScript-->>GitHubActions: CTLD.lua built at repo root
GitHubActions->>Pytest: poetry run pytest
alt CTLD.lua present
Pytest->>Pytest: run all tests (262 passed, 0 skipped)
else CTLD.lua missing
Pytest->>Pytest: [skipif(not CTLD.lua)]
Pytest->>Pytest: tests using .miz installation skipped
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
test_inject_into_miz, theskipifcondition recomputes the repo root viaparents[3]; consider reusing the same repo-root helper/path used by the other CTLD.lua-guarded tests to avoid brittle path assumptions. - The new
python-qualitypath filters (src/**andtools/build/**) significantly broaden when the workflow runs; double-check this won't cause unnecessary CI runs compared to a more targeted subset of build-related files. - Since
merge_CTLD.ps1is now run on Linux viapwsh, it might be worth adding an explicit failure message or check when PowerShell is unavailable to make CI issues easier to diagnose if the runner image changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `test_inject_into_miz`, the `skipif` condition recomputes the repo root via `parents[3]`; consider reusing the same repo-root helper/path used by the other CTLD.lua-guarded tests to avoid brittle path assumptions.
- The new `python-quality` path filters (`src/**` and `tools/build/**`) significantly broaden when the workflow runs; double-check this won't cause unnecessary CI runs compared to a more targeted subset of build-related files.
- Since `merge_CTLD.ps1` is now run on Linux via `pwsh`, it might be worth adding an explicit failure message or check when PowerShell is unavailable to make CI issues easier to diagnose if the runner image changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The skipif guard and the mission fixture each recomputed parents[3]. One REPO constant instead, which is what every other test module in this directory does — there is no shared helper to reuse. CI proved the two acceptance criteria left open: pwsh is on the ubuntu image (`shell: /usr/bin/pwsh`, then 32 files merged) and python-quality reports 262 passed, 0 skipped with CTLD.lua absent from the repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks — one taken, two declined with reasons. 1. 2. Broader 3. Explicit check for a missing For the record, the two acceptance criteria that were left unticked are now proven by this PR's own run: 262 passed, 0 skipped on ubuntu with 🤖 Addressed by Claude Code |
Carried here rather than into a one-line PR of its own, per Zip. The convention is to set the index line inside the lot's own PR; #110 shipped without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…CON-SOUNDS) (#112) * chore(backlog): mark CHORE-UNTRACK-BUILT-ENGINE merged (PR #110) Carried here rather than into a one-line PR of its own, per Zip. The convention is to set the index line inside the lot's own PR; #110 shipped without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(tool): a beacon sound the Mission Maker chooses (FEAT-CUSTOM-BEACON-SOUNDS) radioSound and radioSoundFC3 were text boxes: typing a name changed what the engine plays without putting any such file in the mission, so using your own beacon tone meant editing the .miz by hand — the manual step FEAT-ONE-CLICK-INSTALL exists to remove. Each now gets a Default / Custom picker. A chosen .ogg is read at selection, kept in the session, and written into the archive with its resource key and preload trigger like the bundled ones. The model, per ADR 0012: a chosen file enters the mission under a reserved name (CTLD_beacon_custom.ogg), so the configuration value itself says the sound is customised — no second key that could contradict the engine, and no misreading of a Mission Maker whose own file is called beacon.ogg. The name it had on disk survives as a schema-only label, never catalogued: a catalogue entry would be a parameter under ADR 0011 Addendum 1, so completeness would demand it and every pre-lot configuration would report a missing setting at mission start (FIX-TOOL-I18N-LANG's wall). Reading the bytes at selection rather than at install is what makes an installed mission reconfigurable: reopening the .miz recovers the sound, so it reinstalls on another machine with the original file deleted. A .yaml cannot carry a binary, so reopening one blocks the install with a validation error naming the file to pick again — unless the target mission already holds it. An OggS signature check catches the renamed .mp3 that would give silent beacons discovered in flight; no size cap, the size is reported instead. The picker is bound to `editor: sound` in the schema, never to a setting name in a component (FEAT-EDITOR-COVERAGE), and `hidden: true` keeps the labels out of the families and out of search. Typing a file name by hand still works for a sound added through the Mission Editor. Documented EN + FR, in the tool guide and the configuration reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(backlog): mark FEAT-CUSTOM-BEACON-SOUNDS merged (PR #112) Set in the lot's own PR, per the convention — not left for a post-merge commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Why
CTLD.luais generated bymerge_CTLD.ps1and committed anyway..gitignorecalled it deliberate — "intentionally tracked — available at repo root for DCS missions" — a reason from the repository bootstrap that no longer holds:docs/);vendored.yamlpins2.0.0-rc3withmanual_steps: "(verbatim) re-download the CTLD.lua asset from the matching VEAF/CTLD release"and watchesgithub-release;Meanwhile the cost lands on every code change: 26 of the 28 merges touching
src/over 30 days carried the regenerated file — a one-megabyte generated diff nobody reviews, and a guaranteed conflict between parallel PRs.It is safe now because
FEAT-DEV-BUILD-CHANNEL(#109) publishes the engine where people take it: attached to every release, and to the floatingdevpre-release rebuilt on each merge. Confirmed on that channel's first run —CTLD.luaattached, declaringctld.VERSION = "2.0.0-rc6-182ec25".The trap this PR leads with
python-qualityruns on ubuntu, checks out, and runspytest— it never built the engine. 27 tests guarded byskipif(not CTLD.lua)ran only because the artifact was committed. Measured by removing it:CTLD.luaDeleting the file alone would have left CI green while the whole
.mizinstallation suite stopped running. So the build comes first (ticket 01), the deletion last (ticket 03).What changed
merge_CTLD.ps1runs on Linux. Three literal Windows paths, not the two the ticket listed:generate_i18n_dicts.ps1:36carries the same"..\.."and the merge script calls it. All three now compose one segment at a time. It stays the single build path — a lookalike built for the tests would drift.python-qualitybuilds the engine beforepytest, and itspaths:filter gainssrc/**andtools/build/**, since the job now depends on both.test_inject_into_mizgains the guard its neighbours have. It failed withKeyError: 'injected'when no engine was built — an error naming neither the engine nor the build. WithoutCTLD.luathe suite now reports 234 passed / 28 skipped / 0 failed.git rm --cached CTLD.luaplus a.gitignoreentry replacing the old "do NOT ignore this" comment, next to the precedent already there (/src/CTLD_config_default_yaml.lua, generated the same way).CLAUDE.mdnow says never commit as well as never hand-edit;building-and-testing.{md,fr.md}gain a table saying where to get an engine per role (contributor / Mission Maker / tester).No history rewriting — the 471 past blobs weigh 2.8 MiB packed out of a 93 MiB
.git.What this PR's own CI proves
That
pwshis present on the ubuntu image (the fallback, if not, iswindows-latest) and thatpython-qualityreports 262 passed, 0 skipped with the file absent from the repository. Both acceptance boxes are left unticked until then rather than ticked on a hunch.🤖 Generated with Claude Code
Summary by Sourcery
Untrack the generated CTLD.lua engine file by updating build and CI to create it on demand, tightening tests to handle its absence cleanly, and revising documentation and backlog entries to reflect the new distribution and usage model.
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: