Skip to content

Add WriterT instances for supertypes of Monad - #628

Merged
stew merged 1 commit into
typelevel:masterfrom
ceedubs:writert-instances
Nov 15, 2015
Merged

stew merged 1 commit into
typelevel:masterfrom
ceedubs:writert-instances

Conversation

@ceedubs

@ceedubs ceedubs commented Nov 12, 2015

Copy link
Copy Markdown
Contributor

This is a partial fix for #620. There are still more WriterT instances that could be added, but this is already a sizable PR, so I thought getting in the supertypes of Monad would be a good chunk of work to split off.

I've gone to fairly great lengths to ensure that implicit resolution works in various cases in the tests. Perhaps I've even gone a bit overboard, but I think that these tests are kind of necessary, because there are some inference issues with Id that are compounded by issues with type aliases. In fact I had to rework the prioritization hierarchy several times as these tests uncovered issues.

@codecov-io

Copy link
Copy Markdown

Current coverage is 76.35%

Merging #628 into master will increase coverage by +0.22% as of bc03128

@@            master    #628   diff @@
======================================
  Files          159     159       
  Stmts         2070    2085    +15
  Branches        69      69       
  Methods          0       0       
======================================
+ Hit           1576    1592    +16
  Partial          0       0       
+ Missed         494     493     -1

Review entire Coverage Diff as of bc03128

Powered by Codecov. Updated on successful CI builds.

@non

non commented Nov 14, 2015

Copy link
Copy Markdown
Contributor

👍

1 similar comment
@stew

stew commented Nov 15, 2015

Copy link
Copy Markdown
Contributor

👍

stew added a commit that referenced this pull request Nov 15, 2015
Add WriterT instances for supertypes of Monad
@stew
stew merged commit 3ad2fd6 into typelevel:master Nov 15, 2015
@stew stew removed the in progress label Nov 15, 2015
@ceedubs
ceedubs deleted the writert-instances branch November 15, 2015 23:38
@ceedubs ceedubs mentioned this pull request Nov 18, 2015
19 tasks done
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.

4 participants