-
Notifications
You must be signed in to change notification settings - Fork 2k
chore: bump to rust edition 2024 #10802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
.github/workflows/test.yml
Outdated
@@ -82,7 +82,6 @@ jobs: | |||
- uses: actions/checkout@v4 | |||
- uses: dtolnay/rust-toolchain@nightly | |||
with: | |||
toolchain: nightly-2024-02-03 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found out we were pinning to this specific nightly, this seems unintended / resolved
@@ -46,7 +46,7 @@ pub struct HexIdProvider { | |||
|
|||
impl HexIdProvider { | |||
/// Generates a random hex encoded Id | |||
pub fn gen(&self) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gen
is now a reserved keyword, found generate
better than r#gen
crates/cheatcodes/src/env.rs
Outdated
@@ -20,7 +20,7 @@ impl Cheatcode for setEnvCall { | |||
} else if value.contains('\0') { | |||
Err(fmt_err!("environment variable value can't contain NUL character `\\0`")) | |||
} else { | |||
env::set_var(key, value); | |||
unsafe { env::set_var(key, value); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsafe
is now required for things that touch the environment
let settings = cache.profiles.get(&profile).expect("must be present"); | ||
|
||
settings | ||
cache.profiles.get(&profile).expect("must be present") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clippy lint rule now enforces returning the result of a
let binding from a block
preferring direct returns where possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good, unsure about the additional clones
Closing for now given it is low priority and can be picked up at any point in the future |
…rmatting anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unblocks let-chains
Motivation
Closes: #10734
Solution
Initial commit was done as a trial run with Claude, worked quite well!
Some of the added.clone()
is concerning, will look for alternative solution to deal with ownership. We are mostly dealing withArc
's and the other fields are likely light to clone.Expecting this PR to fail its lint check, doing that in a follow-up here: #10803 as the diff is very large in terms of number of files touchedPR Checklist