refactor: make some options non-exhaustive #27
Conversation
src/agent/mod.rs
Outdated
| /// # let url = Url::parse("https://example.com").unwrap(); | ||
| /// let opts = LoginOptions { | ||
| /// redirect_url: Some(url), | ||
| /// ..Default::default() |
There was a problem hiding this comment.
Functional update syntax doesn't work with non_exhaustive structs, so this won't work (And I'd say it makes constructing options a bit awkward anyway, builder pattern is way nicer).
At this point, I think that giving those fields pub(crate) visibility and thus enforcing builder pattern would lead to less confusing backwards compatible api :)
There was a problem hiding this comment.
Yea, just figured that out …
At this point, I think that giving those fields pub(crate) visibility and thus enforcing builder pattern would lead to less confusing backwards compatible api :)
Good point!
There was a problem hiding this comment.
I am with @kate-shine the builder pattern would be way easier to extend and make it easier to use too.
ctron
force-pushed
the
feature/non_exhaustive_options_1
branch
from
0a66625 to
b794f6e
Compare
BREAKING-CHANGE: In order to allow adding new options in the future, the structs LoginOptions and LogoutOptions have been made non-exhaustive. Adding some documentation on how to work with them.
ctron
force-pushed
the
feature/non_exhaustive_options_1
branch
from
b794f6e to
2217ce0
Compare
BREAKING-CHANGE: In order to allow adding new options in the future,
the structs LoginOptions and LogoutOptions have been made
non-exhaustive. Adding some documentation on how to work with them.
@kate-shine @AlexandreRoba @DerKnerd It would be great if you could give this a quick look. I think this should simplify adding new fields in the future. But it will break the API right now. Which the two pending PRs seem to do anyway. That's fine, but let's take the opportunity to improve this.