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

Revamp type classes doc #1440

Merged
merged 3 commits into from
Nov 16, 2016
Merged

Conversation

adelbertc
Copy link
Contributor

Also removed type class list at bottom of type classes page since it's now in the sidebar

I think the previous version didn't really motivate type classes or discuss how type classes compare to the "usual" subclassing. I think this new one does a better job, but of course am open to comments/criticisms :-)

Also removed type class list at bottom of type classes page since it's now in the sidebar
@codecov-io
Copy link

codecov-io commented Oct 26, 2016

Current coverage is 92.19% (diff: 100%)

Merging #1440 into master will not change coverage

@@             master      #1440   diff @@
==========================================
  Files           242        242          
  Lines          3615       3615          
  Methods        3546       3546          
  Messages          0          0          
  Branches         69         69          
==========================================
  Hits           3333       3333          
  Misses          282        282          
  Partials          0          0          

Powered by Codecov. Last update 25e9628...99a01a8

@adelbertc adelbertc mentioned this pull request Oct 27, 2016
18 tasks
@philwills
Copy link
Contributor

This seems a really significant improvement to me. The Monoid example is much more compelling than Show.

I do wonder whether the first paragraph might be a little off-putting and whether moving it till after a motivating example might make it all seem a bit more welcoming for those not sure what ad-hoc polymorphism means.

@non
Copy link
Contributor

non commented Oct 30, 2016

👍

@philwills I think you might be right. That said, I think I'm leaning toward merging this and then iterating on it in future PRs.

It might be nice (either here or somewhere else) to lay out polymorphism via subtyping, type-casing, and type-classes to help illustrate the concept.

@adelbertc
Copy link
Contributor Author

Yeah the intent with the blurb at the top was to give a description of the domain type classes try to solve. I struggled to come up with an alternative intro and am open to suggestions, though I'm leaning to merging and iterating like @non suggested :-)

@adelbertc
Copy link
Contributor Author

@philwills I woke up this morning, had some coffee, and re-read the introduction and I agree it read weird. Rewrote it a bit, let me know what you think.

@non Could you take a look as well?

Thanks!

@philwills
Copy link
Contributor

@adelbertc that definitely seems much more readable to me.

@longcao
Copy link
Contributor

longcao commented Nov 15, 2016

👍! I think this is great.

I'll also echo what @non said about merging and iterating in #1440 (comment), it also seems it would be beneficial to merge/push docs at a faster (and perhaps more aggressive) pace to get it out onto the web, where it is, to me, admittedly a lot easier to read once it's HTMLified. Not sure how anyone else feels about that, though.

@kailuowang
Copy link
Contributor

👍 LGTM, I also agree we can probably iterate docs faster.

@adelbertc
Copy link
Contributor Author

@kailuowang sounds good, thanks! will merge.. could you also give a quick look at #1442 :D

@adelbertc adelbertc merged commit ba2536c into typelevel:master Nov 16, 2016
@stew stew removed the in progress label Nov 16, 2016
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