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

rename Cartesian to Semigroupal #1961

Merged
merged 7 commits into from
Oct 13, 2017
Merged

Conversation

kailuowang
Copy link
Contributor

fixes #1931 with a deprecated type alias and scalafix
There are two commits in this PR the de6345e is the mechanical rename one - not much to look here.
the f42419b adds a scalafix rule that should be easy to review separately

@codecov-io
Copy link

codecov-io commented Oct 10, 2017

Codecov Report

Merging #1961 into master will not change coverage.
The diff coverage is 86.95%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1961   +/-   ##
=======================================
  Coverage   96.07%   96.07%           
=======================================
  Files         273      273           
  Lines        4538     4538           
  Branches      119      122    +3     
=======================================
  Hits         4360     4360           
  Misses        178      178
Impacted Files Coverage Δ
...a/cats/laws/discipline/ApplicativeErrorTests.scala 100% <ø> (ø) ⬆️
...cats/laws/discipline/CommutativeFlatMapTests.scala 50% <ø> (ø) ⬆️
laws/src/main/scala/cats/laws/ApplyLaws.scala 100% <ø> (ø) ⬆️
.../scala/cats/laws/discipline/ApplicativeTests.scala 100% <ø> (ø) ⬆️
core/src/main/scala/cats/syntax/apply.scala 100% <ø> (ø) ⬆️
...aws/src/main/scala/cats/laws/ApplicativeLaws.scala 100% <ø> (ø) ⬆️
...c/main/scala/cats/laws/discipline/MonadTests.scala 100% <ø> (ø) ⬆️
core/src/main/scala/cats/package.scala 100% <ø> (ø) ⬆️
core/src/main/scala/cats/Composed.scala 95.83% <ø> (ø) ⬆️
.../scala/cats/laws/discipline/AlternativeTests.scala 100% <ø> (ø) ⬆️
... and 24 more

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 8cce29c...80c5c09. Read the comment docs.

LukaJCB
LukaJCB previously approved these changes Oct 10, 2017
edmundnoble
edmundnoble previously approved these changes Oct 11, 2017
"invariant cartesian associativity" -> forAll((fa: F[A], fb: F[B], fc: F[C]) => laws.invariantMonoidalAssociativity(fa, fb, fc))
"invariant semigroupal left identity" -> forAll((fa: F[A], b: B) => laws.invariantMonoidalLeftIdentity(fa, b)),
"invariant semigroupal right identity" -> forAll((fa: F[A], b: B) => laws.invariantMonoidalRightIdentity(fa, b)),
"invariant semigroupal associativity" -> forAll((fa: F[A], fb: F[B], fc: F[C]) => laws.invariantMonoidalAssociativity(fa, fb, fc))
Copy link
Contributor

Choose a reason for hiding this comment

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

"Invariant monoidal left identity"?

## To test scala fix

```bash
sbt ;coreJVM/publishLocal;coreFree/publishLocal
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this redundant with the other scalafix PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you referring the PR that add this to travis? This is to add it to readme so people know how to run the test locally.

object RenameCartesianTests {
import cats.syntax.semigroupal._

def prod[F[_]: Semigroupal, A, B](fa: F[A], fb: F[B]): F[(A, B)] = Semigroupal[F].product(fa, fb)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

}

// ref: https://github.com/typelevel/cats/pull/1961
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@kailuowang kailuowang dismissed stale reviews from edmundnoble and LukaJCB via 4129beb October 11, 2017 13:14
LukaJCB
LukaJCB previously approved these changes Oct 11, 2017
LukaJCB
LukaJCB previously approved these changes Oct 11, 2017
@@ -90,4 +90,7 @@ package object cats {
val Semigroup = cats.kernel.Semigroup
val Monoid = cats.kernel.Monoid
val Group = cats.kernel.Group

@deprecated("renamed to Semigroupal", "1.0.0-RC1")
type Cartesian[F[_]] = Semigroupal[F]
Copy link
Member

Choose a reason for hiding this comment

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

I think you've got some extra white space here

@kailuowang
Copy link
Contributor Author

I'm going to go ahead and merge this. It got 2 sign-offs (1 of which dismissed by minor changes )

@kailuowang kailuowang merged commit 41eaa83 into typelevel:master Oct 13, 2017
@kailuowang kailuowang added this to the 1.0.0-RC1 milestone Oct 13, 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.

Renaming Cartesian to Semigroupal
4 participants