ci: validate board config required fields on PR#9678
Merged
igorpecovnik merged 1 commit intomainfrom Apr 16, 2026
Merged
Conversation
igorpecovnik
requested review from
hzyitc,
iav,
mhoffrog and
rpardini
as code owners
github-actions
bot
added
size/large
igorpecovnik
force-pushed
the
validate-board-configs
branch
from
b74f6e8 to
339ceb8
Compare
github-actions
bot
added
size/medium
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a pull-request-triggered GitHub Actions workflow that detects changed board config files and runs a new Python validator which parses top-level KEY=value assignments, emits ERROR/WARNING findings (and optional GitHub annotations), and exits non‑zero on validation errors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PR as Pull Request
participant GH as GitHub Actions
participant Runner as Actions Runner
participant Git as Git
participant Validator as tools/validate-board-config.py
PR->>GH: open/update PR
GH->>Runner: trigger workflow (path filters)
Runner->>Git: checkout (fetch-depth: 0)
Runner->>Git: git diff --name-only base..head (filter extensions)
Git-->>Runner: changed file list
Runner->>Runner: write filtered paths to GITHUB_OUTPUT
alt no relevant files
Runner->>Runner: log "no board configs changed"
else relevant files present
Runner->>Validator: run `python3 tools/validate-board-config.py --github <files>`
Validator->>Validator: parse assignments & validate fields
Validator->>GH: emit ::warning/::error annotations (if --github)
Validator-->>Runner: exit code (0 or 1)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
igorpecovnik
force-pushed
the
validate-board-configs
branch
2 times, most recently
from
8e9e76c to
af54a0f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/maintenance-validate-board-configs.yml (1)
61-66: Consider passing the file list via env to avoid YAML-level shell interpolation.
${{ steps.changed.outputs.files }}is expanded by the Actions templating engine before bash ever sees it, so any quoting inside that string is moot and a future filename containing shell metacharacters ($, backticks,;) would be interpreted. Board filenames are tame today, but the safer idiom is to read the list fromenv::- name: "Run validator" if: steps.changed.outputs.files != '' + env: + CHANGED_FILES: ${{ steps.changed.outputs.files }} run: | set -euo pipefail - # shellcheck disable=SC2086 - python3 tools/validate-board-config.py --github ${{ steps.changed.outputs.files }} + # shellcheck disable=SC2086 # intentional word-splitting on space-separated list + python3 tools/validate-board-config.py --github $CHANGED_FILES🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/maintenance-validate-board-configs.yml around lines 61 - 66, The workflow currently expands ${{ steps.changed.outputs.files }} directly in the run shell which risks YAML-level interpolation of filenames; instead, set an env entry (e.g. CHANGED_FILES) for the "Run validator" step populated with ${{ steps.changed.outputs.files }} and invoke the validator script (tools/validate-board-config.py) reading from that env var (or passing "$CHANGED_FILES" as the --github argument) so the shell receives a single quoted value rather than pre-expanded, potentially unsafe text; update the "Run validator" step to use env: CHANGED_FILES: ${{ steps.changed.outputs.files }} and reference that env inside the run block.tools/validate-board-config.py (1)
170-174: Totals count includes skipped/non-file inputs.
len(args.files)reports every path passed on the CLI even when it was skipped as “not a file” or “unknown extension”. Tracking the number of files actually validated keeps the summary honest, especially once someone pipes a broadergit difflist into the script.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-board-config.py` around lines 170 - 174, The summary currently prints totals using len(args.files) which includes skipped/non-file inputs; change it to report the number of files actually validated by tracking or deriving a validated count (e.g., count entries you processed successfully or not skipped) and use that instead in the final print; update the print call that references errors, warnings, and args.files to use the validated_files_count (or the existing list/variable that holds validated file paths) so the summary reads f"\n{len(errors)} error(s), {len(warnings)} warning(s) across {validated_count} file(s)".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/maintenance-validate-board-configs.yml:
- Around line 46-48: Update the git diff command that populates the files array:
the current --diff-filter=AM silently skips renames, so change the option to
include rename statuses (e.g. --diff-filter=ACMR or include R) in the mapfile
invocation that runs git diff --name-only --diff-filter=AM "${base}" "${head}"
...; ensure the new filter (ACMR) is used in that same command so renamed board
configs are discovered and validated.
In `@tools/validate-board-config.py`:
- Around line 111-112: The current check only ensures fields.get("BOARDFAMILY")
is non-empty but the error text claims it must match a family directory; update
the validation to enforce that by resolving the value from
fields.get("BOARDFAMILY") and verifying os.path.isdir(os.path.join("config",
"sources", "families", value)); if the directory does not exist call
err("BOARDFAMILY", "...") with a message stating the family was not found,
otherwise keep success; modify the check around fields.get("BOARDFAMILY") and
the err("BOARDFAMILY", ...) call accordingly.
- Around line 74-82: The current naive inline-comment stripping
(value.split("#", 1)[0]) can remove legitimate unquoted content like
COLOR=#deadbeef or URLs with fragments; update the logic that trims inline
comments so it only removes a trailing "#" when it is preceded by whitespace
(bash-like semantics). Locate the block that tests value and not
value.startswith('"')/value.startswith("'") and replace the unconditional split
with a check for a '#' and that the character before the '#' is whitespace (or
use a regex matching '\s#') before taking the left side; leave the
surrounding-quote stripping and out[key] assignment unchanged.
- Around line 104-140: validate() currently omits checks for BOOTCONFIG and
BOOT_FDT_FILE described in the PR; add the same style of validation used for
BOARD_NAME/BOARD_VENDOR: check fields (the fields dict) and call
err("BOOTCONFIG", "...") when BOOTCONFIG is missing or empty for built boards
(i.e. boards not marked as .wip), and call warn("BOOT_FDT_FILE", "...") when
BOOT_FDT_FILE is missing or empty except for .tvb boards (skip the warn for
boards marked .tvb); implement these checks in validate() using the existing
err() and warn() helpers and the board identity logic already present in the
function.
---
Nitpick comments:
In @.github/workflows/maintenance-validate-board-configs.yml:
- Around line 61-66: The workflow currently expands ${{
steps.changed.outputs.files }} directly in the run shell which risks YAML-level
interpolation of filenames; instead, set an env entry (e.g. CHANGED_FILES) for
the "Run validator" step populated with ${{ steps.changed.outputs.files }} and
invoke the validator script (tools/validate-board-config.py) reading from that
env var (or passing "$CHANGED_FILES" as the --github argument) so the shell
receives a single quoted value rather than pre-expanded, potentially unsafe
text; update the "Run validator" step to use env: CHANGED_FILES: ${{
steps.changed.outputs.files }} and reference that env inside the run block.
In `@tools/validate-board-config.py`:
- Around line 170-174: The summary currently prints totals using len(args.files)
which includes skipped/non-file inputs; change it to report the number of files
actually validated by tracking or deriving a validated count (e.g., count
entries you processed successfully or not skipped) and use that instead in the
final print; update the print call that references errors, warnings, and
args.files to use the validated_files_count (or the existing list/variable that
holds validated file paths) so the summary reads f"\n{len(errors)} error(s),
{len(warnings)} warning(s) across {validated_count} file(s)".
🪄 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
Run ID: 8aa79f21-1c79-443a-a235-1f27a31dfcde
📒 Files selected for processing (2)
.github/workflows/maintenance-validate-board-configs.ymltools/validate-board-config.py
igorpecovnik
force-pushed
the
validate-board-configs
branch
from
af54a0f to
ef2df35
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/validate-board-config.py`:
- Around line 113-114: The KERNEL_TARGET check currently only enforces non-empty
value via fields.get("KERNEL_TARGET") and err("KERNEL_TARGET", ...); update this
to verify that the provided value contains at least one token from the allowed
set {"legacy","current","edge","vendor","mainline"} (e.g., split the string on
whitespace/commas and check intersection), and call err("KERNEL_TARGET", ...) if
no allowed token is present; alternatively, if you intend to keep the looser
check, change the error message text to not claim allowed values. Ensure the
validation lives where fields.get("KERNEL_TARGET") is checked so the new logic
replaces the simple non-empty check.
🪄 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
Run ID: 50610855-3068-4510-9218-1c39720f2b36
📒 Files selected for processing (2)
.github/workflows/maintenance-validate-board-configs.ymltools/validate-board-config.py
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/maintenance-validate-board-configs.yml
igorpecovnik
force-pushed
the
validate-board-configs
branch
2 times, most recently
from
bc646fe to
d4019ea
Compare
igorpecovnik
added
Ready to merge
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/maintenance-validate-board-configs.yml:
- Around line 53-66: The "Run validator" step currently interpolates
steps.changed.outputs.files directly into the shell which enables shell
injection and breaks filenames with spaces; instead stop expanding ${{
steps.changed.outputs.files }} inside the run script and consume the output from
the step as a single environment variable (e.g. env: FILES: ${{
steps.changed.outputs.files }}) or read it from GITHUB_ENV, then parse it safely
inside the run block (use readarray/read -d '' for NUL-delimited input or printf
... | xargs -0 to build argv) and pass those safe arguments to the python
invocation (tools/validate-board-config.py) rather than embedding the templated
value inline in the command; update the "Run validator" step to use this safe
consumption of the files output.
🪄 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
Run ID: c7c22895-9dcc-4332-81c2-77853f538f56
📒 Files selected for processing (2)
.github/workflows/maintenance-validate-board-configs.ymltools/validate-board-config.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/validate-board-config.py
Adds a PR-time validator that checks board configs for missing
required / recommended fields. Helps catch issues before merge
rather than at first build attempt by the unlucky maintainer who
picks up the orphan board.
tools/validate-board-config.py:
- Standalone Python validator, runnable locally:
tools/validate-board-config.py config/boards/orangepi5.conf
- Extracts top-level KEY=VALUE assignments via regex (does NOT
source the bash file; boards have side-effecty function bodies)
- Emits plain text findings; with --github also emits ::error
/ ::warning workflow commands that annotate the PR diff
- Exit 1 if any error, else 0
Rule table:
ERRORS:
BOARD_NAME required, non-empty
BOARD_VENDOR required, non-empty
BOARDFAMILY required, must match config/sources/families/
KERNEL_TARGET must contain at least one of:
legacy / current / edge / vendor / mainline
BOOTCONFIG required (except .wip)
WARNINGS:
BOARD_MAINTAINER recommended; orphan boards rot
INTRODUCED recommended; year first shipped
BOOT_FDT_FILE recommended (skip on .tvb)
Per-extension behavior:
.conf / .csc full rule set
.tvb full rule set, skip BOOT_FDT_FILE warning
.wip only BOARD_NAME is error; rest demoted to warning
.eos skipped (dead boards aren't validated)
.github/workflows/maintenance-validate-board-configs.yml:
- Triggers on pull_request touching config/boards/** (or the
validator/workflow itself)
- Detects changed files via git diff against PR base — only
validates what the PR actually touched, not all 500+ boards
- Runs validator with --github so findings show on the PR diff
- Skips cleanly when no board configs changed (other PRs unaffected)
Smoke-tested against orangepi5.conf, odroidn2.conf, aml-t95z-plus.tvb
(real boards, no errors) and synthetic broken .conf / .wip files
(errors and warnings as expected).
igorpecovnik
force-pushed
the
validate-board-configs
branch
from
d4019ea to
34eacaf
Compare
github-actions
bot
added
Needs review
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
1 participant
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.
Summary
Adds a PR-time validator for
config/boards/*.{conf,csc,tvb,wip,eos}files. Catches missing required fields before merge instead of at first build attempt.Two files:
tools/validate-board-config.py— standalone Python validator, runnable locally:tools/validate-board-config.py config/boards/orangepi5.conf tools/validate-board-config.py config/boards/*.confExtracts top-level
KEY=VALUEassignments via regex (handles bare,export, anddeclareprefixes — does not source the bash file). With--githubemits::error/::warningworkflow commands that annotate the PR diff..github/workflows/maintenance-validate-board-configs.yml— runs on PRs touchingconfig/boards/**. Detects changed files viagit diffagainst PR base — only validates what the PR actually touched, not all 500+ boards.Rule table
BOARD_NAMEBOARD_VENDORBOARDFAMILYKERNEL_TARGETBOARD_MAINTAINERINTRODUCEDKERNEL_TEST_TARGETPer-extension behavior
.conf/.csc.tvb.wipBOARD_NAMEis error; rest demoted to warning.eosTest plan
.confboards pass with 0 errorsexport BOARD_NAME=...syntax (uefi-loong64.conf) parsed correctly.confproduces errors, exit 1.wipdemotes errors to warnings exceptBOARD_NAMESummary by CodeRabbit
New Features
Chores