docs: check MSP references are up to date in CI (backport to release/9.1) - #11946
sensei-hacker wants to merge 6 commits into
Conversation
README.md hadn't been regenerated since MSP2_INAV_SET_AUX_RC and MSP2_INAV_WIND were added to msp_messages.json, and since hwVersion's field description was updated for uint8_t encoding - both source changes landed without a matching `gen_docs.sh` run. The new CI check being backported alongside this (PR iNavFlight#11875) diffs a fresh regen against the checked-in copy, so it would fail immediately on release/9.1 without this.
…rompt release/9.1's gen_docs.sh still ends with an interactive `read -n 1 -s -r -p "Press any key to continue"`. Under GitHub Actions' default non-interactive shell (bash -eo pipefail), reading from a closed stdin fails and becomes the script's own exit status, so `bash gen_docs.sh` returns nonzero even when the docs are perfectly up to date - aborting the step (via -e) before the actual staleness check ever runs. Gate the prompt behind a TTY check so it still pauses for a human running it locally but is a no-op in CI. maintenance-10.x's gen_docs.sh has already been rewritten without this prompt, so this only affects the release/9.1 backport.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoEnforce up-to-date MSP reference docs in CI for release/9.1
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11946 245 targets built. Find your board's
|
The TTY-gated prompt fix (previous commit) unconditionally ended the script with `true`, so if any preceding Python generator step failed partway through, the failure was discarded and the script still exited 0 - the CI job's git-status check would then see whatever partial/unchanged state was left and report success despite the docs never having actually been (re)validated. Add `set -euo pipefail` so any real failure aborts immediately, and replace the bare `true` with a proper `if [ -t 0 ]; then read ...; fi` that doesn't need a trailing catch-all to stay -e-safe. Caught by Qodo's review on the PR carrying the previous fix.
|
Qodo's finding on `gen_docs.sh` (generator failures could pass the docs check silently, since the script had no `set -e` and unconditionally ended with `true`) was correct - fixed in 991ac26: added `set -euo pipefail` and replaced the trailing prompt handling with a proper `if [ -t 0 ]; then read ...; fi` instead of the bare fallback. Verified by simulating a generator crash (`python` step exits nonzero) - the script now correctly aborts instead of reaching the end and reporting success. |
|
@xznhj8129 Orginally #11875 against `maintenance-10.x`, from bkleiner, but it needs to get to master, via 9). Since you're the primary maintainer on the MSP reference docs, flagging in case you want to take a look before it merges — it also regenerates the docs here since they'd drifted out of sync on this branch. |
Alright, excellent. I guess i can remove the CI from mine and keep it to the code? |
Oh I didn't realize you had a CI one. If you think the CI you had is better, we can use it. Or we can use this one, or a combination of the two? |
3 participants
Summary
Backport of #11875 onto
release/9.1— cherry-picks its three commits (CI check that MSP reference docs are regenerated and up to date, deterministic enum-generation ordering, and untracked-file detection viagit status --porcelain), since this is a CI/tooling improvement rather than a breaking change and should land as early as possible so it flows forward intomaintenance-10.xandmasternaturally.Two additional commits were needed to make this actually work on
release/9.1, since it diverges frommaintenance-10.x:docs: regenerate stale MSP reference docs on release/9.1—docs/development/msp/README.mdhad drifted out of sync withmsp_messages.jsonbefore this PR (missingMSP2_INAV_SET_AUX_RC/MSP2_INAV_WINDentries, and a stalehwVersionfield description) since nothing enforced regeneration until now. Without this, the new CI check would fail immediately on landing.Fix msp_docs CI job always failing due to gen_docs.sh's interactive prompt—release/9.1'sdocs/development/msp/gen_docs.shstill ends with an interactivereadprompt (already removed inmaintenance-10.x's rewritten version of this script). Under GitHub Actions' default non-interactive shell, thatreadfails on EOF and aborts the step before the actual staleness check runs, which would make the new job permanently red regardless of doc state. Gated the prompt behind a TTY check instead.Changes
msp_docsCI job to.github/workflows/docs.yml, failing ifdocs/development/msp/gen_docs.sh's output differs from the checked-inREADME.md/inav_enums.json/inav_enums_ref.mdget_all_inav_enums_h.py's file iteration deterministic (sorted(...)) so regeneration doesn't flip entries depending on filesystem enumeration ordergen_docs.sh's trailing interactive prompt so it doesn't break the new CI job under a non-interactive shellTesting
msp_docsjob logic locally end-to-end (bash gen_docs.sh+ thegit status --porcelainstaleness check) under GitHub Actions' defaultbash -eo pipefailshell with closed stdin — confirmed it now passes cleanly and is idempotent (re-running produces zero diff)bash gen_docs.shreturns exit 1 on EOF stdin even when docs are fully up to date, and confirmed-eaborts the step before the staleness check ever runsgit status --porcelain(vs.git diff --exit-code, which the original PR's own follow-up commit also moved away from) correctly detects a deleted-then-recreated-as-untracked generated file, by reproducing that exact scenarioCMakeLists.txt,docs/development/msp/*) exist at the same paths onrelease/9.1Code Review
Reviewed with the
inav-code-reviewagent, which independently reproduced the interactive-readCI failure in a scratch worktree before the fix was applied, and confirmed the fix and the regeneration commit afterward. No other issues found.Related
Supersedes/backports #11875 for
release/9.1(original PR targetsmaintenance-10.x; a maintainer should confirm this is later cherry-picked/isn't otherwise superseded once both branches converge).