ci: retire -D warnings, adopt Cargo [lints] as sole lint policy#111
Merged
TigerInYourDream merged 1 commit intomainfrom Apr 21, 2026
Merged
ci: retire -D warnings, adopt Cargo [lints] as sole lint policy#111TigerInYourDream merged 1 commit intomainfrom
TigerInYourDream merged 1 commit intomainfrom
Conversation
5 tasks
…source
Rust stable releases every ~6 weeks and routinely promotes clippy lints
from allow to warn-by-default. Combined with RUSTFLAGS=-D warnings in CI,
unrelated PRs were failing on errors introduced entirely by upstream lint
drift.
Cargo.toml already declares per-lint policy via [lints.rust] and
[lints.clippy] (stable since Cargo 1.74). This commit retires the env-flag
half so that [lints] is the single source of truth.
To preserve the "author's own unused/dead code gets caught in their PR"
behavior that -D warnings was providing, two specific denies are added:
unused = { level = "deny", priority = -1 }
dead_code = "deny"
These are pinned per-lint and immune to upstream drift. The unused lint
group uses priority = -1 so that the existing unused_import_braces = "warn"
override continues to win for that specific lint (required by
clippy::lint_groups_priority, stable since Cargo 1.74).
release.yml already omitted -D warnings, so no release-side changes here.
TigerInYourDream
force-pushed
the
ci/drop-d-warnings
branch
from
6c76478 to
3ff4f4c
Compare
2 tasks
TigerInYourDream
added a commit
that referenced
this pull request
Apr 21, 2026
The original clippy_report job added in 9a02b40 was non-blocking: `continue-on-error: true` plus `|| true` on the cargo invocation, so a dirty clippy state at release time would show up only as a summary panel while the release publish steps still produced artifacts. That turned the job into pure visibility without discipline — warnings could accumulate on main indefinitely and still ship. This commit upgrades the semantics: * Removes `continue-on-error: true` at the job level and `|| true` on the cargo command; any warning or hard error now fails the job. * Splits the single "run + summarize" step into two: the run step is allowed to fail normally, while the summarize step carries `if: always()` so the GitHub step-summary panel and the clippy.log artifact are produced even on failure. Without the split, a dirty run would hide the categorized warning report that makes cleanup tractable. * Adds a final gate check in the summarize step that exits 1 with an `::error::` annotation when warn_count > 0 OR error_count > 0. Belt and suspenders alongside `set -o pipefail`. * Adds `clippy_report` to `create_release.needs` and extends its `if:` expression with `needs.clippy_report.result == 'success'`. Because the surrounding `always()` still evaluates the condition even when upstream jobs are skipped, the `result == 'success'` check is what actually cascades the block. A failed clippy_report skips create_release, which skips every downstream for_* build and publish job — no artifacts are produced with dirty clippy. * Renames the display to `Clippy Gate` (the job key stays `clippy_report` so the Swatinem cache key and upload-artifact name remain unchanged — no CI cache churn). Rationale: the CI policy retirement of `-D warnings` in #111 moved lint enforcement from "every PR, for every lint" to "Cargo.toml per-lint denies for author self-defects". That is intentionally lenient on style/perf/complexity/suspicious clippy warnings, which would otherwise make every Rust toolchain bump a PR-carpet-bombing event. The unavoidable consequence is that those warnings can accumulate on main between releases. The release hard gate is the scheduled cleanup trigger: it forces the maintainer to clear the backlog at the exact moment it matters — before shipping. Locally verified on the branch prior to push: current robrix code compiles clippy-clean (warn_count=0, error_count=0), so this change does not immediately block the next release. The first toolchain bump that promotes a lint from allow to warn in the `unused` or correctness group will fail the bump PR itself (by the layer-2 lint policy); anything lower-priority will surface at the next release and require a deliberate cleanup PR.
TigerInYourDream
added a commit
that referenced
this pull request
Apr 21, 2026
…in mgmt) (#112) * ci: add clippy_report job to release workflow Run cargo clippy on every tag push / workflow_dispatch. Non-blocking: continue-on-error: true at job level plus "|| true" on the cargo invocation. The job writes a categorized warning summary to $GITHUB_STEP_SUMMARY (displayed at top of the Actions run page) and uploads the full clippy log as a 30-day artifact. Purpose: visibility into clippy state at release time, so new warnings introduced by upstream Rust/clippy releases can be reviewed during the regular packaging cycle rather than blocking random PR authors. Warning counts in the summary come from the #[warn(...)] annotations emitted by rustc/clippy (not the top-level "warning:" line), so cargo meta-warnings such as duplicate-package notices do not inflate the count. Companion to the CI policy change that retires RUSTFLAGS=-D warnings in favor of Cargo.toml [lints] as the sole lint policy source. * ci: pin rust-toolchain to 1.94.0 and swap to pin-aware action Switches from floating dtolnay/rust-toolchain@stable to actions-rust-lang/setup-rust-toolchain@v1 across all 14 workflow invocations (main.yml x1, builds.yml x7, release.yml x6). The new action honors rust-toolchain.toml as the single source of truth for the Rust version used by CI and local development. rust-toolchain.toml is pinned to 1.94.0 (current stable). The upgrade process is documented inline as TOML comments, so anyone opening the file to bump the version sees the checklist. Combined with retiring -D warnings and the clippy_report job in the preceding commit, robrix2 moves from passive toolchain drift to active control: CI uses the exact compiler specified in rust-toolchain.toml, local dev auto-installs the same version via rustup, toolchain bumps are deliberate maintainer-opened PRs, and clippy_report at release surfaces the lint state of the pinned toolchain. * ci: disable actions-rust-lang default rustflags to preserve non-strict CI actions-rust-lang/setup-rust-toolchain@v1 defaults to rustflags: "-D warnings" and exports that to $GITHUB_ENV, which silently re-injects strict mode into every subsequent step including cargo install of third-party crates. This was caught on PR #112's first CI run: cargo-makepad itself has an unused_variable warning (ndk_version at compile.rs:368) which normally is just a warning, but got promoted to an error by the injected RUSTFLAGS, failing all 4 mobile builds. Explicitly setting rustflags: "" on all 14 invocations disables the default injection. Lint policy remains entirely in Cargo.toml [lints.rust] and [lints.clippy], which is the whole point of the migration. * ci: promote clippy_report from soft report to hard release gate The original clippy_report job added in 9a02b40 was non-blocking: `continue-on-error: true` plus `|| true` on the cargo invocation, so a dirty clippy state at release time would show up only as a summary panel while the release publish steps still produced artifacts. That turned the job into pure visibility without discipline — warnings could accumulate on main indefinitely and still ship. This commit upgrades the semantics: * Removes `continue-on-error: true` at the job level and `|| true` on the cargo command; any warning or hard error now fails the job. * Splits the single "run + summarize" step into two: the run step is allowed to fail normally, while the summarize step carries `if: always()` so the GitHub step-summary panel and the clippy.log artifact are produced even on failure. Without the split, a dirty run would hide the categorized warning report that makes cleanup tractable. * Adds a final gate check in the summarize step that exits 1 with an `::error::` annotation when warn_count > 0 OR error_count > 0. Belt and suspenders alongside `set -o pipefail`. * Adds `clippy_report` to `create_release.needs` and extends its `if:` expression with `needs.clippy_report.result == 'success'`. Because the surrounding `always()` still evaluates the condition even when upstream jobs are skipped, the `result == 'success'` check is what actually cascades the block. A failed clippy_report skips create_release, which skips every downstream for_* build and publish job — no artifacts are produced with dirty clippy. * Renames the display to `Clippy Gate` (the job key stays `clippy_report` so the Swatinem cache key and upload-artifact name remain unchanged — no CI cache churn). Rationale: the CI policy retirement of `-D warnings` in #111 moved lint enforcement from "every PR, for every lint" to "Cargo.toml per-lint denies for author self-defects". That is intentionally lenient on style/perf/complexity/suspicious clippy warnings, which would otherwise make every Rust toolchain bump a PR-carpet-bombing event. The unavoidable consequence is that those warnings can accumulate on main between releases. The release hard gate is the scheduled cleanup trigger: it forces the maintainer to clear the backlog at the exact moment it matters — before shipping. Locally verified on the branch prior to push: current robrix code compiles clippy-clean (warn_count=0, error_count=0), so this change does not immediately block the next release. The first toolchain bump that promotes a lint from allow to warn in the `unused` or correctness group will fail the bump PR itself (by the layer-2 lint policy); anything lower-priority will surface at the next release and require a deliberate cleanup PR.
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
RUSTFLAGS: "-D warnings"from 8 call sites (main.yml global env + builds.yml × 7 build jobs)unused = { level = "deny", priority = -1 }anddead_code = "deny"toCargo.toml [lints.rust]to preserve the "author self-defect" half of-D warnings' jobrelease.ymlalready omitted-D warnings— unchanged hereWhy
Floating
dtolnay/rust-toolchain@stable× blanket-D warningsis the well-known "time bomb" combo: every ~6 weeks Rust stable can promote a clippy lint fromallowtowarnby default, and the next unrelated PR fails on errors its author did not introduce.Cargo.toml [lints.rust]/[lints.clippy](stable since Cargo 1.74) is the modern replacement — per-lint, manifest-versioned, explicit. This repo already uses it for 6 rustc entries and 8 clippy entries. This PR retires the old env-flag mechanism so[lints]is the single source of truth.The two new denies (
unusedgroup,dead_code) are pinned per-lint, so they catch author-introduced mistakes in the PR's own CI without being affected by upstream lint additions.The
unusedgroup usespriority = -1so the existingunused_import_braces = "warn"declaration continues to win for that specific lint — this is required byclippy::lint_groups_priority(stable since Cargo 1.74) and is the idiomatic shape used in rust-lang/rust, cargo, and tokio.Responsibility split after this PR
unused_*/dead_code[lints.rust]deny entries)[lints.rust] forbidentries (unchanged)Test plan
cargo build --profile fastpassescargo clippy --workspace --all-featurespasses (0 warnings, 0 errors)Companion PR
#112 expands the story: pins
rust-toolchain.tomlto an exact version, swaps CI to a toml-aware toolchain action, adds a non-blockingclippy_reportjob torelease.yml, and documents the upgrade cadence in the toml itself. Together, these two PRs move robrix2 from passive lint/toolchain drift to active control.