Skip to content

Unconditionally cap Rust lints at warn#561

Merged
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:cap-lints
Oct 9, 2025
Merged

Unconditionally cap Rust lints at warn#561
mulkieran merged 1 commit intostratis-storage:masterfrom
mulkieran:cap-lints

Conversation

@mulkieran
Copy link
Copy Markdown
Member

@mulkieran mulkieran commented Oct 9, 2025

No description provided.

@mulkieran mulkieran self-assigned this Oct 9, 2025
@mulkieran mulkieran moved this to In Review in 2025September Oct 9, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 9, 2025

Walkthrough

A global macro was added in mockbuild_test/stratisd.spec to append a Rust compiler flag (--cap-lints=warn) to the build_rustflags used during the build process.

Changes

Cohort / File(s) Summary
Spec build flags
mockbuild_test/stratisd.spec
Added %global build_rustflags %{build_rustflags} --cap-lints=warn to include a warning-level lints cap in Rust build flags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I nibbled flags in spec-land green,
A whisper added, softly seen—
“Cap lints to warn,” the carrots say,
So builds hop smoothly, day by day.
With rustle-rust and gentle cheer,
Our burrow’s builds are crystal clear. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the main change of adding a cap-lints=warn flag to all Rust builds, making it clear that lints are being unconditionally capped at warning level. It is concise and directly reflects the modifications in the spec file without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mulkieran mulkieran marked this pull request as ready for review October 9, 2025 16:17
This will allow compilation to proceed when a new release of Rust
introduces a new compiler lint.

Signed-off-by: mulhern <amulhern@redhat.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
mockbuild_test/stratisd.spec (1)

6-7: Verify that capping lints aligns with CI objectives and consider documenting the rationale.

The --cap-lints=warn flag prevents any Rust lint from failing compilation, which may mask issues that would normally block the build. While this is a common practice in CI environments to ensure builds succeed across different Rust versions with varying lint strictness, it could allow code with potential issues to be built.

Since this is in the mockbuild_test directory and the spec explicitly states "This package should not be used in production," this approach appears reasonable for testing/CI purposes. However, consider the following:

  1. Add a comment explaining why lints are capped (e.g., "Cap lints to prevent CI failures from new Rust versions with stricter lints")
  2. Verify that production builds (if any exist separately) do not use this flag
  3. Ensure that lint warnings are still visible in CI output for developers to review

Apply this diff to add a clarifying comment:

+# Cap lints at warn to prevent CI build failures from stricter lints in newer Rust versions
 %global build_rustflags %{build_rustflags} --cap-lints=warn
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 088d576 and 43acfba.

📒 Files selected for processing (1)
  • mockbuild_test/stratisd.spec (1 hunks)

@mulkieran mulkieran merged commit 575579c into stratis-storage:master Oct 9, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in 2025September Oct 9, 2025
@mulkieran mulkieran deleted the cap-lints branch October 9, 2025 16:51
@mulkieran mulkieran moved this to Done(1) in 2025October Oct 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done(1)

Development

Successfully merging this pull request may close these issues.

1 participant