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

Luka jcb add monoidal to monoid #3

Conversation

kailuowang
Copy link

Sorry I also included master commits. The last two commits are new

sullis and others added 6 commits December 17, 2017 07:03
* sbt-coursier 1.0.0

* sbt-coursier 1.0.0
* Add comparison method in Order companion object

The main goal of this PR was to add syntax for the `Order` and
`PartialOrder` syntax (provided via machinist ops) to address [this comment](typelevel#1867 (comment)).

In the process I noticed that the Order companion object didn't have a
`comparison` function to forward through to the type class instance like
it did for other methods, so I added that.

I did notice that the `Order`/`PartialOrder` syntax does work whether or
not you take the `Order`/`PartialOrder` instances as implicit parameters
to the ops classes. I think that only @non understands quite what's
going on with the machinist syntax macros, so I didn't make the changes
to take away those implicit parameters without knowing the implications.
The tests pass either way, so I think that end-code should work for
users either way.

* Change PartialOrder syntax tests to handle NaN
@@ -135,16 +135,6 @@ private[data] sealed abstract class ConstInstances0 extends ConstInstances1 {
}

private[data] sealed abstract class ConstInstances1 {
implicit def catsConstInvariantMonoidal[C: Monoid]: InvariantMonoidal[Const[C, ?]] = new InvariantMonoidal[Const[C, ?]] {
Copy link
Author

Choose a reason for hiding this comment

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

already provided by Applicative

@LukaJCB LukaJCB merged commit 30391e8 into LukaJCB:add-monoidal-to-monoid Dec 18, 2017
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