Skip to content

Conversation

@RJSonnenberg
Copy link
Contributor

Description

NOTE: It will be easier to review this PR by stepping through each commit.

  • Update Documentation (first commit)
    • Correct typos, grammar, examples, formatting
    • Reword for clarity
    • Add more clarity to some sections
  • Add IValidateable<'F> interface type and documentation (second commit)
  • Add example for using IValidateable interface in concert with Giraffe model validation (third commit)

- Correct typos, grammar, examples, formatting
- Reword for clarity
- Add more clarity to some sections
@RJSonnenberg RJSonnenberg added the enhancement New feature or request label Jul 20, 2023
@RJSonnenberg RJSonnenberg requested a review from devinlyons July 20, 2023 20:24
@RJSonnenberg RJSonnenberg self-assigned this Jul 20, 2023
Username: string option
Password: string option }
interface IValidateable<NewUserFailures> with
Copy link
Contributor

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
Copy link
Contributor

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()
Copy link
Contributor

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.

@RJSonnenberg
Copy link
Contributor Author

Closing this PR in favor of a separate Giraffe model validation interface #17

@devinlyons devinlyons deleted the add-ivalidatable-interface branch November 17, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants