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

instance/syntax traits and binary compatibility #496

Closed
ceedubs opened this issue Aug 29, 2015 · 7 comments
Closed

instance/syntax traits and binary compatibility #496

ceedubs opened this issue Aug 29, 2015 · 7 comments
Milestone

Comments

@ceedubs
Copy link
Contributor

ceedubs commented Aug 29, 2015

Currently we have quite a few places where we have traits that are neither sealed nor private that house instances and syntax. For example the Option instances and the Option syntax.

Adding a new instance or method to these traits is a change that is not binary compatible. Ideally, we could freely add new instances or syntax and not have to worry about waiting until the next minor version to merge them.

Personally I know that I always get my syntax/instances via imports as opposed to mixing in traits. How much do other people value being able to mix these in instead of importing them?

@travisbrown
Copy link
Contributor

How would you support à la carte vs. cats.syntax.all._ if these were objects?

I may have missed some discussion about this, but the project is young enough that it seems like it might be better to bump the minor version after every release (and save patch versions for really bad mistakes) instead of distorting code to ensure 100% binary compatibility (which we explicitly don't guarantee yet, anyway).

@ceedubs
Copy link
Contributor Author

ceedubs commented Aug 29, 2015

@travisbrown I was envisioning the traits being private[cats], which as far as I know would alleviate the binary compatibility issue (please correct me if I'm wrong).

I agree that bumping the minor version is fine for now. This is something I was aiming to address by a first 1.0 RC.

@travisbrown
Copy link
Contributor

@ceedubs Oh, got it. I wouldn't be strongly opposed to making them private[cats]. (Still not sure being able to add syntax in a patch version is an especially good thing anyway, though, but I think I'm more of a syntax skeptic than most people.)

@ceedubs
Copy link
Contributor Author

ceedubs commented Aug 29, 2015

I think I'm more interested in being able to add type class instances without waiting until the next minor version. For example "oops we defined the monad for FooT[F, ?] if F has a monad, but we neglected to define the functor for FooT[F, ?] if F has a functor but not a monad". In the past it's been annoying for me to have to wait 6 months for the next version of scalaz for something like that.

EDIT: to clarify my message, I'm saying that doing the same for syntax isn't as important to me.

@ceedubs ceedubs added this to the cats-1.0.0 milestone Nov 7, 2015
ceedubs added a commit to ceedubs/cats that referenced this issue Nov 7, 2015
This is related to typelevel#496, but it's a broader issue than just this change.

These instances traits/classes are used internally for code organization
and implicit prioritization, but they aren't really meant to be part of
the public API. Making them private/sealed may be helpful for binary
compatibility in the future. It also may be a helpful indicator to
newcomers which traits/classes are actually meant for consumption, so
they can ignore some of the noise.

So far, I have left the traits for orphans from `std` (such as
`StringInstances`), and I have left the omni-instance traits exposed.
@ceedubs
Copy link
Contributor Author

ceedubs commented Nov 22, 2015

@travisbrown at one point on Gitter you mentioned that you've grown fond of having the instance traits exposed. However you said you'd wait until you weren't on your phone to expand upon why. Do you want to weigh in on this issue with your use-case?

@travisbrown
Copy link
Contributor

@ceedubs It's partly just a vague dislike for introducing type class instances with imports, and I won't try to defend that.

The more interesting part is that being able to use the instance / syntax traits means that I can effectively "re-export" syntax, etc. for my own types—e.g. I can mix cats.syntax.SemigroupSyntax into my the companion object for my Enumerator and then anyone can use enum |+| enum without any imports at all (and this happens without muddying up the implicit scope too much).

@ceedubs
Copy link
Contributor Author

ceedubs commented May 14, 2016

I'm going to close this out. I've think we've pretty much settled on a convention somewhere in the middle.

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

No branches or pull requests

2 participants