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 builder ergonomics, remove prefix #56

Merged
merged 7 commits into from
Oct 12, 2023

Conversation

divagant-martian
Copy link
Collaborator

@divagant-martian divagant-martian commented Oct 7, 2023

A couple of improvements for ergonomics

  • For an user of the builder, calling new("v4") provides little value taking into account v4 is the only supported identity scheme. So just implement Default for the builder instead,
  • A more common and friendly pattern is to given access to the BuilderOfX via X::builder since that's an easily avoidable import, so now Enr provides: Enr::builder()
  • In some cases it's desirable to get an empty Enr. This would be EnrBuilder::new().build(&key), there is a friendly shortcut for this Enr::empty
  • Removes the Enr prefix from EnrBuilder this shouldn't be even needed to import thanks to the Enr::builder and Enr::empty

Notes

Tests don't use the most ergonomic versions of the builder in their current form (before this change). Since test setup is most of the time done by copying the setup from a related test, I updated them all to use the most appropriate, ergonomic version of what they had

Also, there is a clippy failure but I don't think having those warnings enabled makes sense:

@AgeManning AgeManning merged commit ee6a8d9 into sigp:master Oct 12, 2023
3 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