Skip to content

[NFC] Fix lints added in new Rust version#177

Open
arthurp wants to merge 5 commits intomainfrom
arthurp/fix-lints
Open

[NFC] Fix lints added in new Rust version#177
arthurp wants to merge 5 commits intomainfrom
arthurp/fix-lints

Conversation

@arthurp
Copy link
Copy Markdown
Contributor

@arthurp arthurp commented Mar 12, 2026

This is a follow-up on #176 which reenables a bunch of lints and fixes code that violate them.

This leaves collapsible_match disabled because it recommends changes that change the behavior of the program (by causing match fall-through that didn't happen before).

Whether or not we want to merge this is up for debate because merging this will cause us to fairly arbitrarily diverge from upstream. Pros: The lints are generally good and will improve the code. Cons: This will create arbitrary conflicts if we try to merge from upstream, however we are not allowed to do blind merges, so that may not matter.

Notes for reviewers:

  • The PR is separated into commits, each containing related fixes, so reviewing by commit will mean you are checking related changes together.
  • The commits include reformatting the code, so hiding whitespace diffs may helps. (Especially for the collapse ifs commit.)

@arthurp arthurp force-pushed the arthurp/fix-lints branch 2 times, most recently from fdec2b5 to 8c9d370 Compare March 12, 2026 22:17
@arthurp arthurp force-pushed the arthurp/fix-lints branch from 8c9d370 to 5ce266e Compare March 12, 2026 22:41
@arthurp arthurp requested a review from ioeddk March 13, 2026 01:15
@arthurp arthurp marked this pull request as ready for review March 13, 2026 01:15
@arthurp arthurp requested a review from a team as a code owner March 13, 2026 01:15
@arthurp arthurp changed the title Fix lints added in new Rust version [NFC] Fix lints added in new Rust version Mar 13, 2026
@arthurp arthurp requested a review from aanyas72 March 13, 2026 15:33
@arthurp
Copy link
Copy Markdown
Contributor Author

arthurp commented Mar 13, 2026

@aanyas72 I've added you as a reviewer since you may learn from just reading rust code, and all I need from reviewing this is to check that the change does not affect behavior.

@arthurp arthurp added the tech debt Known short-comings caused by not doing the work right label Mar 23, 2026
let key = Key::random();
let mac = Aead::new().encrypt(&node.0, &key, &Iv::new_zeroed(), &[], cipher)?;

node_entries.push(MhtNodeEntry { pos, key, mac });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we just inline "start_pos + i" here and avoid the zip? It's a bit harder to understand for no real gain imo

},
),
ObservationQuery::new_filter(|m: &Message| {
if m.id.is_multiple_of(2) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could just do m.id.is_multiple_of(2).then_some(m.id)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tech debt Known short-comings caused by not doing the work right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants