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

Order for writert #3556

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Order for writert #3556

merged 1 commit into from
Aug 12, 2020

Conversation

TimWSpence
Copy link
Member

No description provided.

@TimWSpence
Copy link
Member Author

Dependent on #3555

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2020

Codecov Report

Merging #3556 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3556   +/-   ##
=======================================
  Coverage   91.29%   91.29%           
=======================================
  Files         386      386           
  Lines        8577     8580    +3     
  Branches      240      234    -6     
=======================================
+ Hits         7830     7833    +3     
  Misses        747      747           

@TimWSpence TimWSpence force-pushed the order-for-writert branch 2 times, most recently from 523d446 to d792220 Compare August 7, 2020 19:31
@TimWSpence
Copy link
Member Author

Dependent on #3555

This is now merged so this PR should be ready for review :)

@travisbrown travisbrown self-requested a review August 12, 2020 14:37
Copy link
Contributor

@travisbrown travisbrown left a comment

Choose a reason for hiding this comment

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

👍 from me.

Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

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

This seems like a good change to me!

Comment on lines +324 to +325
def compare(that: WriterT[F, L, V])(implicit Ord: Order[F[(L, V)]]): Int =
Ord.compare(run, that.run)
Copy link
Member

Choose a reason for hiding this comment

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

Is this a normal thing to add directly to the type? I actually don't know what our idiom is here.

@djspiewak djspiewak merged commit 0640b67 into typelevel:master Aug 12, 2020
@travisbrown travisbrown added this to the 2.2.0-RC3 milestone Aug 15, 2020
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