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 Either#toEitherNel extension and friends #2475

Merged
merged 2 commits into from
Sep 30, 2018
Merged

Add Either#toEitherNel extension and friends #2475

merged 2 commits into from
Sep 30, 2018

Conversation

kubukoz
Copy link
Member

@kubukoz kubukoz commented Sep 6, 2018

Also brings Ior#toIorNel.

I'm also pondering submitting an EitherTNel type alias in the cats.data package object, together with conversions from EitherT, but then we'd need to do the same for IorT I guess.

@ceedubs ceedubs self-requested a review September 6, 2018 18:00
@kubukoz
Copy link
Member Author

kubukoz commented Sep 6, 2018

I imagine it's going to get tedious if we have all these helpers for NonEmptySet and NonEmptyChain...

@ceedubs
Copy link
Contributor

ceedubs commented Sep 7, 2018

To add to the permutations: should we be making these for NonEmptyChain now? That seems to be the direction that we are encouraging people to go with #2472. I'm guessing that supporting both would be most helpful (but starts to feel like potential bloat).

@kubukoz
Copy link
Member Author

kubukoz commented Sep 7, 2018 via email

@catostrophe
Copy link
Contributor

Agree with @ceedubs - the whole codebase should be reviewed and each *Nel function should get its *Nec pair. I tried to switch to Nec in my project via "replace all" but couldn't due to lack of some functions.

Copy link
Contributor

@easel easel left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable addition. I agree it would be good to add the equivalent Nec methods, not sure it needs to block this one though.

@kubukoz
Copy link
Member Author

kubukoz commented Sep 27, 2018

Created #2539 to track NEC equivalents. I suggest we proceed with this one as it is :)

Copy link
Contributor

@ceedubs ceedubs left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@kubukoz
Copy link
Member Author

kubukoz commented Sep 30, 2018

Can we merge? It'll unblock #2543 which will conflict with this

@LukaJCB LukaJCB merged commit 15f4e59 into typelevel:master Sep 30, 2018
@kubukoz
Copy link
Member Author

kubukoz commented Sep 30, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants