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.

Sign up for GitHub

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

Jump to bottom

feat: Implement overriding API methods and correct inconsistent naming #47

Merged
merged 2 commits into from Dec 10, 2022
Merged

feat: Implement overriding API methods and correct inconsistent naming #47

merged 2 commits into from Dec 10, 2022

Conversation

LeoniePhiline
Copy link

@LeoniePhiline LeoniePhiline commented Dec 6, 2022 •
edited

This is a successor to #17 and the preferred alternative to #46.

This PR

@LeoniePhiline LeoniePhiline mentioned this pull request Dec 6, 2022
Closed
@LeoniePhiline LeoniePhiline mentioned this pull request Dec 6, 2022
Open
tshepang
tshepang reviewed Dec 7, 2022
View reviewed changes
dotenv/src/iter.rs Outdated Show resolved Hide resolved
tshepang
tshepang reviewed Dec 7, 2022
View reviewed changes
dotenv/src/lib.rs Outdated Show resolved Hide resolved
sonro
sonro requested changes Dec 7, 2022
View reviewed changes
Copy link
Collaborator

@sonro sonro left a comment

Choose a reason for hiding this comment

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

Please remove () from function names in the documentation - for consistency with other Rust projects.

Example: from_path_override() becomes from_path_override.

@LeoniePhiline LeoniePhiline force-pushed the fix/overload-override branch 2 times, most recently from 392628a to 1384768 Compare December 7, 2022 23:17
@LeoniePhiline
Copy link
Author

@sonro PTAL

Changes: https://github.com/allan2/dotenvy/compare/392628a9610705d0948c0191ce520f2dc5189935..138476842bb73e423358e10943040461de94a1dd

sonro
sonro approved these changes Dec 10, 2022
View reviewed changes
Copy link
Collaborator

@sonro sonro left a comment

Choose a reason for hiding this comment

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

Very good. Thank you for the changes.

@github-actions
Copy link

Code Coverage

@sonro sonro merged commit 69a0623 into allan2:master Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tshepang tshepang tshepang left review comments

@sonro sonro sonro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@LeoniePhiline @tshepang @sonro