Skip to content

Conversation

@fischeti
Copy link
Contributor

@fischeti fischeti commented Jan 19, 2026

Bumps the Rust edition from 2018 to 2024, and modernizes the project structure in the following way:

  • A new lib.rs file collects all the modules (previously done in main.rs), which is the new entry point of bender. This technically means that bender can now also be used as a library crate and not only a binary crate.
  • main.rs is moved to bin/bender.rs and calls the bender library now (with the bender:: prefix)
  • All extern crate are removed (not required anymore since the 2018 edition).
  • cmd/mod.rs is replaced with cmd.rs, which collects all modules in cmd

Furthermore, new Rust editions typically change some formatting rules and some new clippy warnings surfaced (e.g. unused functions), which were fixed.

@fischeti fischeti force-pushed the fischeti/rust-2024-edition branch 4 times, most recently from fae8f42 to 8450bcd Compare January 23, 2026 15:34
Copy link
Member

@micprog micprog left a comment

Choose a reason for hiding this comment

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

Some comments to help my understanding of the changes.

Comment on lines +6 to +7
eprintln!("{}", e);
std::process::exit(1);
Copy link
Member

Choose a reason for hiding this comment

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

Is this part needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You still need to return a a non-zero exit code somehow. You could also return a result instead, which will actually do the same thing:

fn main() -> Result<(), Box<dyn std::error::Error>> {
    bender::cli::main()?;
    Ok(())
}

However, the error will be debug formatted which is a bit uglz

@@ -5,8 +5,6 @@
//!
//! This module implements the subcommands of the command line tool.
#![deny(missing_docs)]
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to remove this? Documentation is good to have...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually no, I will revert this

Comment on lines -1172 to 1177
/// The currently picked version.
pick: Option<usize>,
/// The available version options. These are indices into `versions`.
options: Option<IndexSet<usize>>,
}

Copy link
Member

Choose a reason for hiding this comment

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

Do I understand correctly these fields are not used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. clippy returns a warning in the new edition

}
});
dep_refs_and_revs.and_then(move |(refs, revs)| {
dep_refs_and_revs.map(move |(refs, revs)| {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure this is the correct way to handle this... There may be a simpler way.

)
.await
.and_then(move |path| {
.inspect(move |&path| {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is the correct way to handle this. If I'm not mistaken, this is a remnant from the original futures implementation, that was updated with async/await, but can probably be simplified now.

}
})
.and_then(move |manifest| {
.inspect(move |&manifest| {
Copy link
Member

Choose a reason for hiding this comment

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

See above comments.

@fischeti fischeti force-pushed the fischeti/rust-2024-edition branch from 8450bcd to 3655b98 Compare January 26, 2026 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants