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

Dedup from_str implementations for Env #426

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Conversation

g-re-g
Copy link
Contributor

@g-re-g g-re-g commented Feb 1, 2023

Not sure if this is wanted or needed, was reading through the code to get a better understanding of how leptos works under the hood and saw some duplication. Figured I'd edit it and put up a PR. :) I'm happy to change it however folks want / delete the PR if it's not needed or wanted!

Some nice things:

  • Only one implementation
  • Unifies error messages
  • Easier to see what's different between the usages

@gbj
Copy link
Collaborator

gbj commented Feb 1, 2023

@akesson @benwis Could you take a look at this? It seems fine to me but you're the experts.

@benwis
Copy link
Contributor

benwis commented Feb 2, 2023

LGTM, it's a fairly straightforward cleanup. Thanks for doing this

@akesson
Copy link
Contributor

akesson commented Feb 2, 2023

Looks good to me as well

@gbj gbj merged commit cbfb724 into leptos-rs:main Feb 2, 2023
gbj pushed a commit that referenced this pull request Mar 21, 2023
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.

None yet

4 participants