build: validate PRs and mirror release matrix in CI#12
Conversation
Three issues with the previous CI: - Triggered only on push-to-main and workflow_dispatch, so PRs were never validated before merge — code only got verified after it had already landed. PR #5 was effectively merged unchecked. - Built on ubuntu-latest (currently ubuntu-24.04) while releases target both ubuntu-22.04 and ubuntu-24.04. A version-specific glibc/libudev issue could break the release without warning. - No clippy or rustfmt at all, so style and obvious bugs went uncaught. Add `pull_request` trigger, build matrix on the same OSes the release matrix uses, and a separate lint job running clippy + fmt. The lint job is `continue-on-error: true` for now because the codebase has 41 clippy warnings and isn't currently fmt-clean — making them blocking would force a large cleanup PR. They'll show up in the checks tab so maintainers see the trend, and a follow-up PR can flip them to blocking once the warnings are addressed.
There was a problem hiding this comment.
Pull request overview
Updates CI to validate pull requests and align CI build targets with the release workflow’s Linux OS matrix, while adding non-blocking lint signal.
Changes:
- Add
pull_requesttrigger to run CI checks pre-merge. - Replace single
ubuntu-latestbuild with a matrix build onubuntu-22.04andubuntu-24.04. - Add an informational
lintjob runningcargo fmt --checkandcargo clippy.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Install packages | ||
| run: sudo apt install libatk1.0-dev pkg-config libgtk-3-dev libudev-dev | ||
|
|
There was a problem hiding this comment.
This apt install step may block on an interactive confirmation prompt and can fail on runners without an updated package index. In CI it's more reliable to run an update first and install with -y (and usually prefer apt-get for scripting). Consider applying the same pattern to the build job as well.
There was a problem hiding this comment.
Fixed in 0384285 — both jobs now run apt-get update followed by apt-get install -y … so they refresh the index and never block on a prompt.
| @@ -19,5 +26,28 @@ jobs: | |||
| - name: Install Rust | |||
| run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | |||
There was a problem hiding this comment.
After installing Rust via rustup, the workflow doesn't add $HOME/.cargo/bin to PATH, so subsequent steps may end up using whatever cargo is preinstalled on the runner (or fail if none is present). Export the rustup-installed toolchain location (e.g., via GITHUB_PATH) so cargo build reliably uses the intended toolchain.
| run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | |
| run: | | |
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | |
| echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
Fixed in 0384285 — moot now: switching to dtolnay/rust-toolchain@stable handles GITHUB_PATH for us, so the build job actually uses the toolchain we set up instead of whatever the runner had pre-installed.
| - name: Install Rust | ||
| run: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | ||
|
|
There was a problem hiding this comment.
Piping a remote script into sh (curl ... | sh) makes the CI environment dependent on an unpinned external script at runtime. For better supply-chain security and reproducibility, consider using a dedicated action to install Rust (or pin the toolchain via a checked-in rust-toolchain.toml and a setup action).
There was a problem hiding this comment.
Fixed in 0384285 — both jobs now use dtolnay/rust-toolchain@stable instead of piping sh.rustup.rs into sh. No more unpinned external script at runtime.
Three issues flagged on PR #12: - apt installs lacked `-y` and didn't refresh the index, so they could block on a prompt or hit a stale cache. Switched to `apt-get update` + `apt-get install -y` for both jobs. - The build job ran `curl ... | sh -s -- -y` to install Rust but never exported `$HOME/.cargo/bin` to `GITHUB_PATH`. Subsequent steps were silently using whatever cargo the runner had pre-installed instead of the freshly installed toolchain — coincidentally fine, but wrong. - Piping a remote script into sh ties CI to an unpinned external endpoint at every run. Both jobs now use `dtolnay/rust-toolchain@stable`, the de-facto standard action used by the Rust project itself. Net effect: CI is shorter, reproducible, and consistent across both build and lint jobs.
Summary
The current CI only triggers on push-to-main/manual dispatch, builds a single ubuntu-latest image, and runs no lints. This PR addresses each:
pull_requesttrigger so changes get a green CI check before being merged into main, instead of after.ubuntu-latestto a matrix ofubuntu-22.04+ubuntu-24.04, the same OSes release.yaml targets. Avoids the kind of version-specific surprise (e.g. missinglibudev-dev, glibc differences) that PR feat: wireless stack flashing for STM32WB5xxG #5 hit.lintjob runningcargo fmt --checkandcargo clippy --release --locked --all-targets.The lint job is informational (
continue-on-error: true) for now: there are currently 41 clippy warnings and several files are not fmt-clean, so making it blocking would force a big cleanup commit. The signal still shows up in the Checks tab so we can address it in a follow-up PR and flip the flag to blocking once it's clean.Test plan
pull_requesttrigger fires).Build (ubuntu-22.04)andBuild (ubuntu-24.04)complete withcargo build --release --locked.Lint (informational)job runs and exits green even with non-zero clippy/fmt counts.Follow-ups (not in this PR)
cargo fmt --allonce.continue-on-error: trueso lint becomes blocking.