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

Libify validation #7

Merged
merged 8 commits into from
Jul 25, 2024
Merged

Libify validation #7

merged 8 commits into from
Jul 25, 2024

Conversation

bitner
Copy link
Contributor

@bitner bitner commented Jul 25, 2024

Merge in changes from #4

gadomski and others added 3 commits July 25, 2024 08:45
- Returns `Result<()>` instead of bools from validation method
- Removes `validate_str`
- Adds crate-specific error based on `thiserror`
- Adds some doctests
@bitner bitner requested a review from gadomski July 25, 2024 19:27
@bitner
Copy link
Contributor Author

bitner commented Jul 25, 2024

@gadomski I tried to merge most of the stuff you had in taking into account the changes I had made, I think that I conceptually at least got most of the stuff you had done in there, but there may be some other things that will need another PR. I'll wait for the thumbs up from you to merge this one.

@bitner
Copy link
Contributor Author

bitner commented Jul 25, 2024

@gadomski The inline testing of the doc examples is crazy slick!

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@bitner bitner requested a review from gadomski July 25, 2024 21:00
@bitner bitner merged commit d34e793 into main Jul 25, 2024
1 check passed
@bitner bitner deleted the libify-validation branch July 25, 2024 21:21
@gadomski
Copy link
Collaborator

@bitner fysa I'm working on another PR that will change the as_ methods to to_ (among a couple other things).

@bitner
Copy link
Contributor Author

bitner commented Jul 25, 2024

@gadomski I'm done working on this for the week, so no worries about any more crossed PRs!

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