Skip to content
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

Consider making the builders infallible #372

Open
Veetaha opened this issue Sep 10, 2024 · 0 comments
Open

Consider making the builders infallible #372

Veetaha opened this issue Sep 10, 2024 · 0 comments

Comments

@Veetaha
Copy link

Veetaha commented Sep 10, 2024

Hi, thank you for the awesome crate ❤️

I'd like to propose an improvement to the vergen API. It currently uses derive_builder to generate builders for various config structs. As a replacement, I propose using the bon crate.

Motivation

The problem with the builders generated by derive_builder are the following:

  • The build() methods of the builders return a Result, which usually requires the users to call unwrap() on it or somehow else get rid of the error, which is an annoyance. In contrast, bon generates fully compile-time-checked builders. Forgetting to set a field triggers a compile error with a readable error message.
  • derive_builder performs no checks for unintentional overwrites, while bon validates that you don't accidentally set the same field twice at compile time.
  • derive_builder doesn't generate a T::builder() method, instead, it requires the users to use TBuilder::default() to create the builder.
  • bon generates more ergonomic API that allows evolving it without breaking changes by default (See the compatibility page for all the possible ways to evolve your builder API in backwards-compatible way).
  • bon allows for greater flexibility by allowing you to generate a builder from a function or an associated method. It's even possible to switch between derive(Builder) and #[builder] on the method named new without breaking changes.

A comparison table between builder crates can be seen on this page


Would you be interested in this change? I understand that this will require a major release, so I understand if you'd like to push back on this, or postpone doing it until any other reasons for the major release arise.

In any case I can submit a PR from my side if you think it's fine to do it.

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

No branches or pull requests

1 participant