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

Add `Validation` data type to Extra modules #100

Closed
vrom911 opened this issue Oct 11, 2018 · 5 comments

Comments

Projects
4 participants
@vrom911
Copy link
Member

commented Oct 11, 2018

It's not in base, but in either package:

Though you don't usually want to drag all that dependencies, so maybe it makes sense to reimplement it in some Extra module?

@chshersh

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

@vrom911 Completely agree with this 👍 💯 You probably don't want to depend on either package in lightweight libraries or applications. But Validation data type is extremely useful. And since we have Extra modules, it's relatively easy to add known and common things to the relude without worrying about name clashes.

@kahlil29

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

Would this include a reimplementation of Validation with new enhancements/features or we would just reuse the current data declaration and instances from Data.Either ?

@chshersh

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

@kahlil29 Couple hours ago I've received an unexpected email from @mauriciofierrom with the attempt to resolve this issue. Here is the gist:

I will duplicate my comments to this solution here:

  1. You can also add Bifoldable and Bitraversable instances. These typeclasses are already in base and we're going to reexport them as well. See relevant issues:
  2. It would be really nice to use -XInstanceSigs extension and all type signatures to all methods in all instances. In relude we want our package to be as beginner-friendly as possible, and having such type signatures significantly increases code readability. See example in this module:
  3. Also, usually, when you're writing general purpose library, it's a good idea to implement all methods from the typeclasses. Default implementations might not be efficient.
  4. In addition to previous point: adding {-# INLINE #-} pragma to every implemented method helps performance as well.
  5. Also, in relude we're using 4 spaces for indentation :)
@mauriciofierrom

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2018

@chshersh thanks for the comments. If it's ok with you (and @kahlil29) I will work on this issue then.

@kahlil29

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

@chshersh Noted :)
@mauriciofierrom Go ahead 👍

mauriciofierrom added a commit to mauriciofierrom/relude that referenced this issue Oct 15, 2018

mauriciofierrom added a commit to mauriciofierrom/relude that referenced this issue Oct 15, 2018

mauriciofierrom added a commit to mauriciofierrom/relude that referenced this issue Oct 15, 2018

mauriciofierrom added a commit to mauriciofierrom/relude that referenced this issue Oct 15, 2018

#2: Hacktoberfest (October, 2018) automation moved this from To do to Done: Issues Oct 16, 2018

chshersh added a commit that referenced this issue Oct 16, 2018

[#100] Add Validation data type to Extra modules (#106)
* [#100] Add Validation data type to Extra modules

* Most changes as per code review

* More changes as per code review

* Change liftA2 inline pragma location

* Add sequenceA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.