Make board_level = release explicit instead of relying on an empty value - #11305
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (100)
🚧 Files skipped from review as they are similar to previous changes (98)
📝 WalkthroughWalkthroughThe CI matrix generator now requires valid ChangesBoard-Level CI Classification
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bin/generate_ci_matrix.py`:
- Around line 49-62: Fix the indentation in the board_level validation block
containing the if statement checking board_level not in BOARD_LEVELS, the print
statement with the error message, and the exit call, ensuring each nesting level
uses exactly four spaces. Also re-indent the board_level filtering blocks in the
subsequent code sections to maintain consistent four-space indentation
throughout all related blocks. Run Flake8 to verify E111 and E114 errors are
resolved before merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e01b739-bf88-4064-8350-282f6410d159
📒 Files selected for processing (99)
.github/copilot-instructions.mdbin/generate_ci_matrix.pyvariants/esp32/chatter2/platformio.inivariants/esp32/diy/hydra/platformio.inivariants/esp32/diy/v1/platformio.inivariants/esp32/m5stack_core/platformio.inivariants/esp32/m5stack_coreink/platformio.inivariants/esp32/nano-g1-explorer/platformio.inivariants/esp32/nano-g1/platformio.inivariants/esp32/radiomaster_900_bandit/platformio.inivariants/esp32/radiomaster_900_bandit_micro/platformio.inivariants/esp32/radiomaster_900_bandit_nano/platformio.inivariants/esp32/rak11200/platformio.inivariants/esp32/station-g1/platformio.inivariants/esp32/tbeam/platformio.inivariants/esp32/tlora_v2_1_16/platformio.inivariants/esp32/tlora_v3_3_0_tcxo/platformio.inivariants/esp32c3/heltec_hru_3601/platformio.inivariants/esp32c6/m5stack_unitc6l/platformio.inivariants/esp32s3/CDEBYTE_EoRa-S3/platformio.inivariants/esp32s3/ELECROW-ThinkNode-G3/platformio.inivariants/esp32s3/ELECROW-ThinkNode-M2/platformio.inivariants/esp32s3/ELECROW-ThinkNode-M5/platformio.inivariants/esp32s3/elecrow_panel/platformio.inivariants/esp32s3/hackaday-communicator/platformio.inivariants/esp32s3/heltec_capsule_sensor_v3/platformio.inivariants/esp32s3/heltec_sensor_hub/platformio.inivariants/esp32s3/heltec_v4/platformio.inivariants/esp32s3/heltec_v4_r8/platformio.inivariants/esp32s3/heltec_vision_master_e213/platformio.inivariants/esp32s3/heltec_vision_master_e290/platformio.inivariants/esp32s3/heltec_vision_master_t190/platformio.inivariants/esp32s3/heltec_wireless_paper/platformio.inivariants/esp32s3/heltec_wireless_tracker/platformio.inivariants/esp32s3/heltec_wireless_tracker_v2/platformio.inivariants/esp32s3/heltec_wsl_v3/platformio.inivariants/esp32s3/m5stack_cardputer_adv/platformio.inivariants/esp32s3/m5stack_cores3/platformio.inivariants/esp32s3/meshnology-w10/platformio.inivariants/esp32s3/meshnology-w12/platformio.inivariants/esp32s3/mini-epaper-s3/platformio.inivariants/esp32s3/picomputer-s3/platformio.inivariants/esp32s3/rak3312/platformio.inivariants/esp32s3/rak_wismesh_tap_v2/platformio.inivariants/esp32s3/seeed-sensecap-indicator/platformio.inivariants/esp32s3/seeed_xiao_s3/platformio.inivariants/esp32s3/station-g2/platformio.inivariants/esp32s3/station-g3/platformio.inivariants/esp32s3/t-beam-1w/platformio.inivariants/esp32s3/t-beam-bpf/platformio.inivariants/esp32s3/t-deck-pro-v1_1/platformio.inivariants/esp32s3/t-deck-pro/platformio.inivariants/esp32s3/t-deck/platformio.inivariants/esp32s3/t-eth-elite/platformio.inivariants/esp32s3/t-watch-s3/platformio.inivariants/esp32s3/t5s3_epaper/platformio.inivariants/esp32s3/tbeam-s3-core/platformio.inivariants/esp32s3/tlora-pager/platformio.inivariants/esp32s3/tlora_t3s3_epaper/platformio.inivariants/esp32s3/tlora_t3s3_v1/platformio.inivariants/esp32s3/tracksenger/platformio.inivariants/esp32s3/unphone/platformio.inivariants/nrf52840/ELECROW-ThinkNode-M1/platformio.inivariants/nrf52840/ELECROW-ThinkNode-M3/platformio.inivariants/nrf52840/ELECROW-ThinkNode-M4/platformio.inivariants/nrf52840/ELECROW-ThinkNode-M6/platformio.inivariants/nrf52840/ELECROW-ThinkNode-M8/platformio.inivariants/nrf52840/canaryone/platformio.inivariants/nrf52840/diy/nrf52_promicro_diy_tcxo/platformio.inivariants/nrf52840/feather_diy/platformio.inivariants/nrf52840/heltec_mesh_node_t096/platformio.inivariants/nrf52840/heltec_mesh_node_t1/platformio.inivariants/nrf52840/heltec_mesh_pocket/platformio.inivariants/nrf52840/heltec_mesh_solar/platformio.inivariants/nrf52840/heltec_mesh_tower_v2/platformio.inivariants/nrf52840/muzi_base/platformio.inivariants/nrf52840/nano-g2-ultra/platformio.inivariants/nrf52840/r1-neo/platformio.inivariants/nrf52840/rak2560/platformio.inivariants/nrf52840/rak3401_1watt/platformio.inivariants/nrf52840/rak4631_epaper/platformio.inivariants/nrf52840/rak4631_eth_gw/platformio.inivariants/nrf52840/rak4631_nomadstar_meteor_pro/platformio.inivariants/nrf52840/rak_wismeshtag/platformio.inivariants/nrf52840/rak_wismeshtap/platformio.inivariants/nrf52840/seeed_solar_node/platformio.inivariants/nrf52840/seeed_wio_tracker_L1_eink/platformio.inivariants/nrf52840/seeed_xiao_nrf52840_kit/platformio.inivariants/nrf52840/t-echo-lite/platformio.inivariants/nrf52840/t-echo/platformio.inivariants/nrf52840/t-impulse-plus/platformio.inivariants/nrf52840/wio-sdk-wm1110/platformio.inivariants/nrf52840/wio-tracker-wm1110/platformio.inivariants/rp2040/rak11310/platformio.inivariants/rp2040/rp2040-lora/platformio.inivariants/rp2040/rpipico/platformio.inivariants/rp2040/seeed_xiao_rp2040/platformio.inivariants/rp2350/rpipico2/platformio.inivariants/rp2350/seeed_xiao_rp2350/platformio.ini
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
2933ddb to
185f492
Compare
Relying on board_level = <empty> was causing some inheritence footguns. Let's be explicit about what's being released.
185f492 to
478cba4
Compare
Resolves conflicts from #11305 which made board_level explicit. - Keep PR's sophisticated select_changed logic in generate_ci_matrix.py - Update EMITTABLE_LEVELS to use 'release' instead of None - Add board_level validation in load_all_envs (fail fast on missing/unknown levels) - Update build_outlist to match 'release' explicitly instead of not-set - Update tests to use level='release' for release boards - Accept auto-merged board_level = release additions to ~100 variant files
Relying on board_level = <empty> was causing some inheritence footguns. Let's be explicit about what's being released.
What & why
generate_ci_matrix.pytreated absence ofboard_levelas "this is a release board". That sentinel is an inheritance footgun: an env's level was decided by whatever it inherited (or failed to inherit) throughextends, so a variant could silently land in — or drop out of — the release matrix without anything in itsplatformio.inisaying so, and forgetting the key entirely was indistinguishable from deliberately choosing release.This makes the release level explicit. Every variant env now declares one of
pr,release, orextra, and the matrix generator refuses to run if any env is missing the key or uses an unrecognized value.What changed
variants/**/platformio.ini(97 files, +122 lines): addedboard_level = releaseto every env that had no level of its own and inherited none. Existingboard_level = prandboard_level = extravalues were left untouched. These are pure one-line additions — no other content changed in any variant file.bin/generate_ci_matrix.py:BOARD_LEVELSconstant documenting the three levels.board_level == "release"instead ofnot board_level.board_level, naming the offending env. This is the point of the change — a new variant can no longer omit the key and get a level by accident.checkpath into an equivalent single condition..github/copilot-instructions.md: documented all three levels and the new hard requirement. Also corrected a stale claim thatcustom_meshtastic_support_levelcontrols PR-vs-merge builds — it does not; nothing reads it for matrix filtering. It is variant metadata thatbin/platformio-custom.pyemits assupportLevelin the generated hardware list.Behavior
The generated CI matrix is unchanged. This is a refactor of how the release level is spelled, not which boards get built. The only intended behavior change is the new fail-fast validation, which cannot trigger on this branch precisely because all 249 envs now declare a level.
Testing
Built two git worktrees —
develop(before) and this branch (after) — and diffed the generator's output across 14 targets (all 12variants/platform dirs plusallandcheck) × 4 level forms (bare,--level pr,--level extra,--level pr extra) = 56 invocations per tree, 112 runs. Each pair was compared both as an exact stdout string and as an order-insensitive board set.All 56 pairs are byte-for-byte identical — not merely same-set-different-order, which would still have reshuffled CI job ordering.
allall --level prall --level extraall --level pr extracheckcheck --level prcheck= 80 andcheck --level extra= 80 both match, which specifically confirms the rewrittencheckpath still keepsextra-levelboard_checkenvs in the non-PR check matrix.The new validation was verified separately by running the updated script against
develop's un-migrated variants, where it correctly fails:Also confirmed PlatformIO already strips trailing whitespace and inline
;comments from the value, soboard_level = release ; noteparses fine and needs no special handling.Reviewer notes
t5s3-epaper-v1andt5s3-epaper-v2invariants/esp32s3/t5s3_epaper/platformio.ini. Their section headers carry trailing comments ([env:t5s3-epaper-v1] ; H752), which defeats a naive^\[env:...\]$match. Both are included here; the post-change audit confirms all 249 envs resolve to exactly one level (122release/ 108extra/ 19pr), zero missing.src/ortest/is touched — there is no runtime/firmware change, so there is no device-level regression surface.--levelstill accepts onlyprandextra. Addingreleaseas a choice would be redundant (the bare form already means "pr + release") and ambiguous in combinations like--level pr release.🤝 Attestations
On-device testing is not applicable: this changes only CI matrix metadata and the matrix generator. No firmware source is modified, and the produced build matrix is byte-identical to
develop, so every board builds exactly as it did before.🤖 Generated with Claude Code
Summary by CodeRabbit
CI Improvements
Documentation