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 some basic docs for Foldable #473

Merged
merged 3 commits into from
Aug 29, 2015
Merged

Conversation

philwills
Copy link
Contributor

@adelbertc's comment in #460 made me realise that Foldable doesn't currently have any docs.

Once again, these are largely based off the Scaladoc, with a few examples added.

This was based on the branch for #460, but if for some reason, you don't want that and do want this, I'm happy to rebase.

@adelbertc
Copy link
Contributor

Could you add a section on a the monoid for product/tuple types? Not sure if you should put it on Foldable or Monoid or both (maybe mention it in Monoid and then give an example in Foldable) - its fairly common (esp. in data processing contexts) to be able to compute multiple "summary" values over a large set of data in a single pass (think streaming data).

@non
Copy link
Contributor

non commented Aug 21, 2015

This is great, thanks!

I have an ask which is smaller than @adelbertc's: we should make sure to point out that the built-in signature of List#foldRight is different than Cats'. For now this could just be a sentence but I think it's worth mentioning.

That all said, I'm also happy to merge this now and then update the document in future PRs. 👍

@codecov-io
Copy link

Current coverage is 62.90%

Merging #473 into master will increase coverage by +0.12% as of e959c39

@@            master   #473   diff @@
=====================================
  Files          154    154       
  Stmts         2359   2359       
  Branches        66     66       
  Methods          0      0       
=====================================
+ Hit           1481   1484     +3
  Partial          0      0       
+ Missed         878    875     -3

Review entire Coverage Diff as of e959c39

Powered by Codecov. Updated on successful CI builds.

@non
Copy link
Contributor

non commented Aug 24, 2015

👍

I'd be happy to merge this now and expand on it later. Thanks @philwills!

@fthomas
Copy link
Member

fthomas commented Aug 24, 2015

Could you replace "typeclass" with "type class" since we have recently agreed on that spelling (#441).

Other than that, LGTMT and I'm also fine with merging it now.

@ceedubs
Copy link
Contributor

ceedubs commented Aug 25, 2015

It looks like this PR includes the same monoid doc that is in #460. Is that an accident?

It would probably be good to add some clarification that "Right" vs "Left" is really about association as opposed to starting at the end vs beginning of the data structure. That is, we can implement exists, forall, etc in terms of foldRight on an infinite stream, and they can still terminate (depending on the values in the stream). However, with documentation I'm usually pretty inclined to get something out there and people can expand upon it over time, so I give a 👍 for this once the monoid thing is sorted out.

@philwills
Copy link
Contributor Author

@ceedubs I've rebased against master, so that this is clean of the changes from #460.

I'm not sure I understand the distinction around right/left being about association well enough to write about it yet, but might do some more digging/thinking, then give it a go.

@ceedubs
Copy link
Contributor

ceedubs commented Aug 29, 2015

The Travis error is unrelated.

I can't speak for the CSS change, but the rest looks good to me.

stew added a commit that referenced this pull request Aug 29, 2015
Add some basic docs for Foldable
@stew stew merged commit 4f09336 into typelevel:master Aug 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants