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 reducible docs #1777

Merged
merged 3 commits into from
Oct 17, 2017
Merged

Add reducible docs #1777

merged 3 commits into from
Oct 17, 2017

Conversation

LukaJCB
Copy link
Member

@LukaJCB LukaJCB commented Aug 1, 2017

Looking for some early feedback on this, still mostly unstructured :)

@codecov-io
Copy link

codecov-io commented Aug 1, 2017

Codecov Report

Merging #1777 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1777   +/-   ##
=======================================
  Coverage   96.09%   96.09%           
=======================================
  Files         273      273           
  Lines        4554     4554           
  Branches      129      122    -7     
=======================================
  Hits         4376     4376           
  Misses        178      178

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7a0204...66b0795. Read the comment docs.

@kailuowang kailuowang modified the milestone: 1.0.0-RC1 Aug 8, 2017
@LukaJCB LukaJCB mentioned this pull request Aug 8, 2017
70 tasks
@LukaJCB LukaJCB changed the title WIP: Add reducible docs Add reducible docs Aug 25, 2017
@LukaJCB LukaJCB force-pushed the add-reducible-docs branch 2 times, most recently from 2c34aea to 8f8c764 Compare August 25, 2017 07:25
@kailuowang kailuowang modified the milestone: 1.0.0-RC1 Aug 29, 2017
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.

Thanks @LukaJCB! I left a couple of minor/nitpicky comments, but I'm happy with this PR being merged with or without addressing them.

`Reducible` extends the `Foldable` type class with additional `reduce` methods.

You may have come by one of the `reduce`, `reduceLeft` or `reduceOption` defined in Scala's standard collections.
`Reducible` offers exactly these methods with the guarantee, that the collection won't throw an exception at runtime, without having to reduce to an `Option`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: I don't think that you need a comma after "guarantee".

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that years of Java have made me paranoid. Maybe "the collection won't throw an exception at runtime" is a bit of a strong guarantee. Maybe we should just say that these methods won't throw an exception due to a collection being empty? :)

ceedubs
ceedubs previously approved these changes Sep 21, 2017
@kailuowang
Copy link
Contributor

Seems history got messed up?

@LukaJCB
Copy link
Member Author

LukaJCB commented Oct 1, 2017

Fixed!

@kailuowang
Copy link
Contributor

Sorry I dropped ball on this one. @LukaJCB for some reason Github conflict merge tool is not working for us. Do yo mind update this one and merge?

kailuowang
kailuowang previously approved these changes Oct 17, 2017
@kailuowang kailuowang dismissed stale reviews from ceedubs and themself via 66b0795 October 17, 2017 16:02
@LukaJCB
Copy link
Member Author

LukaJCB commented Oct 17, 2017

I think it's fine actually, no? You guys just need to re-review to merge :)

@kailuowang kailuowang merged commit 4c9189e into typelevel:master Oct 17, 2017
@LukaJCB LukaJCB deleted the add-reducible-docs branch October 17, 2017 20:55
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.

4 participants