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.

Sign up for GitHub

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

Jump to bottom

Add more group-like methods to NonEmptyList & Chain #3680

Merged
merged 11 commits into from
Feb 5, 2021
Merged

Add more group-like methods to NonEmptyList & Chain #3680

merged 11 commits into from
Feb 5, 2021

Conversation

BalmungSan
Copy link
Contributor

@BalmungSan BalmungSan commented Nov 12, 2020 •
edited
Loading

Scope:

  • NonEmptyList#groupMap
  • NonEmptyList#groupMapNem
  • NonEmptyList#groupMapReduceWith
  • NonEmptyList#groupMapReduceWithNem
  • NonEmptyList#groupMapReduce (requires a Semigroup)
  • NonEmptyList#groupMapReduceNem (requires a Semigroup)
  • Chain#groupMap
  • NonEmptyChain#groupMapNem
  • Chain#groupMapReduceWith
  • NonEmptyChain#groupMapReduceWithNem
  • Chain#groupMapReduce (requires a Semigroup)
  • NonEmptyChain#groupMapReduceNem (requires a Semigroup)

@BalmungSan BalmungSan marked this pull request as ready for review November 13, 2020 01:05
@BalmungSan BalmungSan changed the title WIP: Add more group-like methods to List, NonEmptyList & Chain Add more group-like methods to List, NonEmptyList & Chain Nov 13, 2020
@BalmungSan BalmungSan changed the base branch from 2.2.x to master November 19, 2020 23:31
@larsrh larsrh self-requested a review February 5, 2021 07:45
@larsrh
Copy link
Contributor

larsrh commented Feb 5, 2021

Sorry for the long wait, I will take a look at this PR.

@BalmungSan
Fixing the documentaion of the List extensions
362009c 
Fixed a typo in the documentation code snippets of
List#groupMapReduce & List#groupMapReduceWith
@BalmungSan
Copy link
Contributor Author

Hi, @larsrh thanks for looking into this!

I have fixed the documentation error.
AFAIK this now passes all checks in 2.12, it doesn't in 2.13 due to the naming clash with the stdlib (which I asked about above ).

Let me know what I can do to fix that :)

@larsrh
Copy link
Contributor

larsrh commented Feb 5, 2021

My suggestion to move forward on this: could you remove all the extensions to the stdlib List from this PR and open another one? This one should only contain the ones for NonEmptyList and Chain. Then, we can discuss how to proceed for the smaller subset in the other PR and merge this one without conflicts.

@BalmungSan BalmungSan changed the title Add more group-like methods to List, NonEmptyList & Chain Add more group-like methods to NonEmptyList & Chain Feb 5, 2021
@BalmungSan
Fixing typos in the documentation
9ced12e 
Replacing strip for trim in the code snippets of
Chain, NonEmptyChain & NonEmptyList
@BalmungSan
Copy link
Contributor Author

BalmungSan commented Feb 5, 2021 •
edited
Loading

@larsrh

could you remove all the extensions to the stdlib List from this PR

Done and the build is green now!

and open another one?

I will when we merge this to avoid some rebasing.
Done: #3770

larsrh
larsrh suggested changes Feb 5, 2021
View reviewed changes
Copy link
Contributor

@larsrh larsrh left a comment

Choose a reason for hiding this comment

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

I have only one point regarding the renamed parameters (it applies to all instances).

core/src/main/scala/cats/data/Chain.scala Outdated Show resolved Hide resolved
@larsrh larsrh added this to the 2.4 milestone Feb 5, 2021
@larsrh
Copy link
Contributor

larsrh commented Feb 5, 2021

Thanks a lot, merging this.

@larsrh larsrh merged commit 4c9857c into typelevel:master Feb 5, 2021
@BalmungSan BalmungSan deleted the add-more-group-methods-list branch February 5, 2021 21:00
@BalmungSan BalmungSan mentioned this pull request Feb 5, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larsrh larsrh larsrh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.4
Development

Successfully merging this pull request may close these issues.
2 participants
@BalmungSan @larsrh