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

UUID instances #1399

Merged
merged 4 commits into from
Oct 24, 2016
Merged

UUID instances #1399

merged 4 commits into from
Oct 24, 2016

Conversation

durban
Copy link
Contributor

@durban durban commented Oct 1, 2016

  • Order[UUID]
  • Show[UUID]

@codecov-io
Copy link

Current coverage is 91.69% (diff: 100%)

Merging #1399 into master will increase coverage by <.01%

@@             master      #1399   diff @@
==========================================
  Files           240        242     +2   
  Lines          3610       3614     +4   
  Methods        3546       3550     +4   
  Messages          0          0          
  Branches         63         63          
==========================================
+ Hits           3310       3314     +4   
  Misses          300        300          
  Partials          0          0          

Powered by Codecov. Last update a392654...0c3f58a

@codecov-io
Copy link

codecov-io commented Oct 1, 2016

Current coverage is 91.69% (diff: 100%)

Merging #1399 into master will increase coverage by <.01%

@@             master      #1399   diff @@
==========================================
  Files           240        242     +2   
  Lines          3610       3614     +4   
  Methods        3546       3546          
  Messages          0          0          
  Branches         63         67     +4   
==========================================
+ Hits           3310       3314     +4   
  Misses          300        300          
  Partials          0          0          

Powered by Codecov. Last update a392654...6bf5a18

implicit val catsKernelStdOrderForUUID: Order[UUID] = new UUIDOrder
}

class UUIDOrder extends Order[UUID] {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if you would be willing to add fromComparable to object Order similar to the Show.fromToString, and use it here. So, it would be a method like:

object Order {
  def fromComparable[T <: Comparable]: Order[T] = new Order[T] {
   ...
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, hopefully I'll have some time for that during the weekend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 6bf5a18.

package object uuid extends UUIDInstances

trait UUIDInstances {
implicit val catsKernelStdOrderForUUID: Order[UUID] = new UUIDOrder
Copy link
Contributor

Choose a reason for hiding this comment

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

if you do the below, you can do = Order.fromComparable[UUID]

@johnynek
Copy link
Contributor

johnynek commented Oct 8, 2016

👍

@travisbrown travisbrown mentioned this pull request Oct 18, 2016
13 tasks
@peterneyens
Copy link
Collaborator

peterneyens commented Oct 24, 2016

Thanks @durban, can you add a uuid object to the (cats-core) instances package object ?

@non
Copy link
Contributor

non commented Oct 24, 2016

I agree with @peterneyens. Otherwise, LGTM too! Thanks!

@peterneyens peterneyens merged commit 79e2f69 into typelevel:master Oct 24, 2016
@durban durban deleted the topic/uuidInstances branch December 4, 2016 12:06
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.

5 participants