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

CommutativeMonoid instance for SortedMap #2047

Merged
merged 3 commits into from
Nov 26, 2017

Conversation

alonsodomin
Copy link

This PR adds a CommutativeMonoid instance for SortedMap when there is a CommutativeSemigroup instance for the value type.

Besides adding a test for CommutativeMonoid[SortedMap[String, Int]], I also modified the test for Monoid[SortedMap[String, Int]] to be a Monoid[SortedMap[String, String]], such that I can ensure that the Monoid instance can still be resolved when there is not a CommutativeSemigroup for the value type.

@LukaJCB
Copy link
Member

LukaJCB commented Nov 26, 2017

You'll need to add an exception Mima exception, but otherwise this looks great, thank you!

johnynek
johnynek previously approved these changes Nov 26, 2017
@codecov-io
Copy link

codecov-io commented Nov 26, 2017

Codecov Report

Merging #2047 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2047      +/-   ##
==========================================
+ Coverage   95.07%   95.07%   +<.01%     
==========================================
  Files         311      311              
  Lines        5255     5256       +1     
  Branches      121      126       +5     
==========================================
+ Hits         4996     4997       +1     
  Misses        259      259
Impacted Files Coverage Δ
core/src/main/scala/cats/instances/sortedMap.scala 95.31% <100%> (+0.07%) ⬆️

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 563f612...e8025a6. Read the comment docs.

@LukaJCB LukaJCB merged commit d621581 into typelevel:master Nov 26, 2017
@LukaJCB
Copy link
Member

LukaJCB commented Nov 26, 2017

Awesome, thank you!

@alonsodomin alonsodomin deleted the sortedmap_commutativemonoid branch November 26, 2017 22:04
@kailuowang kailuowang added this to the 1.0.0 milestone Dec 6, 2017
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.

5 participants