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

Make a bunch of classes final and a few private #622

Merged
merged 2 commits into from
Nov 15, 2015

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented Nov 11, 2015

Probably best to default to final and private until we run into a
reason to expose these for extension.

Probably best to default to `final` and `private` until we run into a
reason to expose these for extension.
@codecov-io
Copy link

Current coverage is 78.87%

Merging #622 into master will increase coverage by +0.22% as of 594f46f

@@            master    #622   diff @@
======================================
  Files          159     159       
  Stmts         2108    2116     +8
  Branches        70      72     +2
  Methods          0       0       
======================================
+ Hit           1658    1669    +11
  Partial          0       0       
+ Missed         450     447     -3

Review entire Coverage Diff as of 594f46f

Powered by Codecov. Updated on successful CI builds.

@non
Copy link
Contributor

non commented Nov 12, 2015

Looks good to me. 👍

implicit def prodApplicative[F[_], G[_]](implicit FF: Applicative[F], GG: Applicative[G]): Applicative[Lambda[X => Prod[F, G, X]]] = new ProdApplicative[F, G] {
def F: Applicative[F] = FF
def G: Applicative[G] = GG
}
}

sealed abstract class ProdInstance3 extends ProdInstance4 {
implicit def prodApply[F[_], G[_]](implicit FF: Apply[F], GG: Apply[G]): Apply[Lambda[X => Prod[F, G, X]]] = new ProdApply[F, G] {
private[data] sealed abstract class ProdInstance3 extends ProdInstance4 { implicit def prodApply[F[_], G[_]](implicit FF: Apply[F], GG: Apply[G]): Apply[Lambda[X => Prod[F, G, X]]] = new ProdApply[F, G] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nitpicky but do you mind re-inserting the line-break here for the sake of consistency and the diff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I didn't mean to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm weird. This is on two lines for me locally. Am I hitting some bizarre LF/CR/CRLF inconsistency?

@travisbrown
Copy link
Contributor

👍 from me.

stew added a commit that referenced this pull request Nov 15, 2015
Make a bunch of classes final and a few private
@stew stew merged commit 4ff969e into typelevel:master Nov 15, 2015
@ceedubs ceedubs deleted the finalize-classes branch November 15, 2015 23:37
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.

5 participants