-
Notifications
You must be signed in to change notification settings - Fork 0
[BOB-380] Add IValidateable interface #16
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
Conversation
- Correct typos, grammar, examples, formatting - Reword for clarity - Add more clarity to some sections
| Username: string option | ||
| Password: string option } | ||
| interface IValidateable<NewUserFailures> with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the point of the interface. What's it for?
| type NewUserVM = | ||
| // ... nothing new here | ||
| interface IModelValidation<NewUserVM> with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation suggests validation without transformation. That's antithetical to the design of this library.
| match | ||
| // To access Validate() on the IValidateable<NewUserFailures> interface, | ||
| // we must upcast to that interface. | ||
| (this :> IValidateable<NewUserFailures>).Validate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IValidatable method doesn't really do anything here that a simple function call couldn't handle.
|
Closing this PR in favor of a separate Giraffe model validation interface #17 |
Description
NOTE: It will be easier to review this PR by stepping through each commit.
IValidateable<'F>interface type and documentation (second commit)IValidateableinterface in concert with Giraffe model validation (third commit)