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

added instances of BitSet to allInstances #1592

Merged
merged 3 commits into from
Apr 8, 2017

Conversation

kailuowang
Copy link
Contributor

Fixes #1485 , I double checked. I believe that BitSet is the only missing one.

@kailuowang kailuowang added this to the cats-1.0.0 milestone Apr 7, 2017
@codecov-io
Copy link

codecov-io commented Apr 7, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1592      +/-   ##
==========================================
+ Coverage   92.52%   92.52%   +<.01%     
==========================================
  Files         249      250       +1     
  Lines        3974     3975       +1     
  Branches      131      138       +7     
==========================================
+ Hits         3677     3678       +1     
  Misses        297      297
Impacted Files Coverage Δ
core/src/main/scala/cats/instances/bitSet.scala 100% <100%> (ø)

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 1601570...0ea5608. Read the comment docs.

trait BitSetInstances extends cats.kernel.instances.BitSetInstances {
implicit def catsStdShowForBitSet: Show[BitSet] = new Show[BitSet] {
def show(fa: BitSet): String =
fa.toIterator.map(_.show).mkString("BitSet(", ", ", ")")
Copy link
Contributor

Choose a reason for hiding this comment

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

why would we use show here vs fa.toString? There is nothing generic about BitSet and the current implementation is just going to be the same as toString no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

duh, I wasn't thinking when I mechanically copied from set instances.

@johnynek
Copy link
Contributor

johnynek commented Apr 7, 2017

👍

@kailuowang kailuowang merged commit 40194da into typelevel:master Apr 8, 2017
@kailuowang kailuowang deleted the more-kernal-instances branch April 9, 2017 05:26
@kailuowang kailuowang modified the milestone: 1.0.0-MF Apr 26, 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.

4 participants