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

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see the point! The current setup drops the preceding "error:" in red, if I'm not mistaken, so some adjustment is still needed... Is this where using anyhow or thiserror could be useful?

}
});
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 2 times, most recently from 3655b98 to 8e4b633 Compare January 27, 2026 16:05
@fischeti fischeti force-pushed the fischeti/rust-2024-edition branch from 8e4b633 to 6ffb29c Compare January 27, 2026 16:22
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