Repository navigation
Build, test and lint on push - #285
Merged
Merged
Conversation
Until now nothing did. `.github/workflows` held one file and it only ran cargo-deny, so every PR's test evidence was a human pasting local output, and a linter had never run at all. What that cost, all found within an hour of installing clippy for the first time: a test that had lost its `#[test]` attribute and never ran, `#[test]` applied twice to another, `#[cfg(test)]` twice on a module, 189 clippy warnings across 153 sites including a real bug (`tip.max(N).min(N)` is always the constant), three doc comments damaged by pasted section headers, and 296 files drifted from rustfmt. The matrix builds both the default and `smtp` configurations. That is not symmetry for its own sake. `cargo clippy --fix` removed a `mut` that only the smtp build needs -- the `sinks.push` it guards sits behind `#[cfg(feature = "smtp")]` -- so a default build compiled and all 2,046 tests passed while `--features smtp`, which is what the production node runs, would not compile at all. A single-configuration gate would have shipped that. Clippy runs with `--all-targets` for the same reason: the one genuine bug it found was in an example, which a bare `cargo clippy` does not lint. `RUSTFLAGS: -D warnings` is set globally because most of the real findings were plain rustc warnings -- an unused import, an unreachable statement -- rather than clippy lints. Rustfmt is advisory. 296 files differ; formatting them is a deliberate commit of its own, and folding it in here would bury this change in ~1,800 lines of churn. Reporting the count keeps it visible and shrinking rather than silently growing. Pinned to the toolchain in rust-toolchain.toml. Note that the existing cargo-deny job logs that the same toolchain is not installed, so that file is not honoured there either. Closes #282 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`continue-on-error` stops a failing job failing the run, but the check itself still reports red -- so the first run showed "Rustfmt (advisory) fail", which is exactly what the advisory marking was meant to avoid. A check that is expected to be red is worse than no check. This repository already has a permanently-failing cargo-deny job that everyone has learned to scroll past; a second one makes a real failure easier to miss, because red stops carrying information. The step now captures the count, writes it to the job summary, and exits 0. Green, with a number that can be watched shrinking. Promote it to gating -- drop the `|| true` and the `continue-on-error` -- once the formatting commit lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Until now nothing did.
.github/workflowsheld one file and it only ran cargo-deny, so every PR's test evidence was a human pasting local output, and a linter had never run at all.Closes #282.
What that cost
All found within an hour of installing clippy for the first time:
test_rsk_message_rlp_transactionshad no#[test]attribute and had never run (Clear the clippy backlog, and what it was hiding #281)#[test]applied twice to another test;#[cfg(test)]twice on a moduletip.max(N).min(N)is always the constant, soscan_addrcomputed a tip and discarded it (Fix what clippy's deny-level lints found, and the warnings that meant something #279)Why the matrix builds two configurations
Not symmetry for its own sake.
cargo clippy --fixremoved amutthat only the smtp build needs — thesinks.pushit guards sits behind#[cfg(feature = "smtp")]. A default build compiled and all 2,046 tests passed, while--features smtpwould not compile at all.That is the configuration the production node runs. A single-configuration gate would have shipped it.
Jobs
cargo build --locked+cargo test --locked, matrix{default: --workspace}and{smtp: -p rustock-cli --features smtp}--workspace --all-targets -- -D warnings, plus the same for--features smtpcargo fmt --all --check, reporting how many files differcontinue-on-error)--all-targetsbecause the one genuine bug clippy found was in an example, which a barecargo clippydoes not lint.RUSTFLAGS: -D warningsglobally, because most of the real findings were plain rustc warnings — an unused import, an unreachable statement — rather than clippy lints.Why fmt is advisory
296 files differ. Formatting them is a deliberate commit of its own; folding it in would bury any change in ~1,800 lines of churn. Reporting the count keeps it visible and shrinking rather than silently growing. Flip
continue-on-erroroff once that commit lands.Expected first run
The gate passes on
mainas of #281:The
fmtjob will report ~296 files and stay green by design.Note: the existing cargo-deny job logs
override toolchain '1.93.1-x86_64-unknown-linux-musl' is not installed, sorust-toolchain.tomlis not honoured there. This workflow pins the toolchain explicitly instead.🤖 Generated with Claude Code