Skip to content

Address clippy warnings#603

Merged
kpfleming merged 11 commits into
mainfrom
clippy
Apr 24, 2026
Merged

Address clippy warnings#603
kpfleming merged 11 commits into
mainfrom
clippy

Conversation

@kailan
Copy link
Copy Markdown
Member

@kailan kailan commented Apr 20, 2026

For as long as I can remember, cargo clippy has produced a huge amount of warnings in this project. This pull request resolves as many of those warnings as possible, with a few exceptions:

  • Large size difference between enum variants
  • Functions with too many arguments
  • Complex type definition

I felt these were out of scope for a large sweeping change like this so I've annotated them to be ignored by clippy.

As a result, clippy output is now clean, and it can be added to the CI workflow to ensure new changes are checked for best practices.

@kailan kailan requested a review from a team April 20, 2026 10:15
@kailan kailan added ci Relates to continuous integration maintenance Maintenance and gardening labels Apr 20, 2026
@kailan kailan changed the title Clippy Address clippy warnings Apr 20, 2026
kpfleming
kpfleming previously approved these changes Apr 21, 2026
Copy link
Copy Markdown
Member

@kpfleming kpfleming left a comment

Choose a reason for hiding this comment

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

Overall there are a ton of improvements here, including use of if-let instead of single-arm match, use of let chains instead of nested if blocks, and similar things. I've left a couple of comments on changes that I'm not sure I fully understand and which will need review from others.

There are probably more improvements like this to be had if we allow Viceroy to be built with a newer version of Rust, it's still pinned to 1.90.

Comment thread src/wiggle_abi/req_impl.rs
Comment thread cli/tests/integration/body.rs
Comment thread cli/tests/integration/body.rs
@kailan
Copy link
Copy Markdown
Member Author

kailan commented Apr 21, 2026

There are probably more improvements like this to be had if we allow Viceroy to be built with a newer version of Rust, it's still pinned to 1.90.

I switched to Rust 1.95 and ran clippy there, which found a few more improvements that I included in this PR. For making that upgrade, I have #604 open which is based on this branch.

@cceckman-at-fastly
Copy link
Copy Markdown
Contributor

Generally looks good.

I'd rather get the ignored lints fixed than allowed; #607 addresses those.

Address Clippy lint on variant size by boxing in NextRequest
@kpfleming kpfleming enabled auto-merge April 24, 2026 13:24
@kpfleming kpfleming merged commit 699f3a1 into main Apr 24, 2026
14 checks passed
@kpfleming kpfleming deleted the clippy branch April 24, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Relates to continuous integration maintenance Maintenance and gardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants