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

Improve PackageBuilder #269

Merged
merged 2 commits into from
Nov 17, 2024
Merged

Conversation

oscartbeaumont
Copy link
Contributor

@oscartbeaumont oscartbeaumont commented Nov 17, 2024

Changes:

  • Ensure optional field are not required using builder(default). The documentation implies this can be put on the struct instead of each field but in practice that requires Package: Default which we probally don't want.
  • Add a PackageBuilder::new method which takes all required fields and returns a PackageBuilder. This is optional as PackageBuilder::default does work it provides some extra safety to downstream users as they will get compile errors if required fields are added in a future major.

One implication of adding PackageBuilder::new is that adding a new required field will requiring modifying the new method which is a breaking change requiring a major version. Technically in the old setup you could be add a non-required field without breaking the compliation of downstream crates due to non_exhaustive but in practice it would have been a breaking change to the runtime behavior so it would never have been okay anyway.

Closes my comments in #158.

@oscartbeaumont oscartbeaumont marked this pull request as ready for review November 17, 2024 12:14
@oli-obk oli-obk merged commit c7bee38 into oli-obk:main Nov 17, 2024
6 checks passed
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.

2 participants