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

Impl From<&str> for Namespace instead of FromStr as it can't fail #76

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

alexsnaps
Copy link
Member

Namespace implements FromStr from the stdlib. But afaict Namespace is a String, as such any String represents a valid Namespace. FromStr can fail tho. Even if our implementation had type Err = core::convert::Infallible it still means you have to call unwrap() on the Result from the the try_into() call.

There might have been a reason for this, but given the existing infallible From<String> implementation, I can't see it.

This changes this to impl From<&str> for Namespace, just as it already did for From<String> and makes the .into() -> Namespace removing the intermediary Result that was always a success anyways. As such it removes a bunch of .unwrap() from the code base.

Part one of more to come...

@alexsnaps alexsnaps changed the title Impl From<&str> for Namespace, it can't fail Impl From<&str> for Namespace instead of FromStr as it can't fail Jun 8, 2022
Copy link
Contributor

@rahulanand16nov rahulanand16nov left a comment

Choose a reason for hiding this comment

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

Always good to reduce unwraps 🙏

@alexsnaps alexsnaps merged commit 51591cc into main Jun 9, 2022
@alexsnaps alexsnaps deleted the try_into branch June 9, 2022 12:35
@alexsnaps alexsnaps added this to the Limitador v0.3 milestone Jun 17, 2022
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