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

Bring back Xor for 0.8.0 and remove in 0.9.0 #1332

Closed
adelbertc opened this issue Aug 25, 2016 · 11 comments
Closed

Bring back Xor for 0.8.0 and remove in 0.9.0 #1332

adelbertc opened this issue Aug 25, 2016 · 11 comments
Assignees

Comments

@adelbertc
Copy link
Contributor

adelbertc commented Aug 25, 2016

Gitter chat: https://gitter.im/typelevel/cats?at=57bf20c05bdd197c1cbe93a0

Actual removal: #1310

Proposal (if I understand) is to bring back just Xor and XorT (not including their uses in other parts of the codebase like it was in ApplicativeError) for 0.8.0, mark it with @deprecated, and actually remove in 0.9.0.

Pinging people who have expressed an opinion about this: @travisbrown @non @johnynek @milessabin @rossabaker

@non
Copy link
Contributor

non commented Aug 25, 2016

Sure. I really want to apologize for repeatedly misunderstanding what the plan was here.

I have a proposal -- I think that our minor releases should have a (rotating) release manager who signs-off on these kinds of things, and who is responsible for managing the scope of what will (or won't) change between releases.

I don't think this person has to sign-off on every PR, but things that dramatically change, break, or remove APIs would need this person's sign-off.

@johnynek
Copy link
Contributor

I don't care too much, but if we said we would remove after 0.8.0, it is good to do what we say, I think.

@kailuowang
Copy link
Contributor

I agree with @non's proposal. I added a "Breaking change" label and a "0.8.0" milestone to make it easier to manage releases. My intention was that for issues proposing breaking changes, we can add these labels and milestones so that incoming users can have a better view of what next release involves. The release manager can own such labels and milestones.

@adelbertc
Copy link
Contributor Author

Against
@milessabin https://gitter.im/typelevel/cats?at=57bf2576757a871757b4ac53
@mpilquist https://gitter.im/typelevel/cats?at=57bf25d2ce157d1b57a4e574
@beefyhalo https://gitter.im/typelevel/cats?at=57bf2644757a871757b4af60

For
@rossabaker https://gitter.im/typelevel/cats?at=57bf26f2f066bd731b4a5719
@tpolecat https://gitter.im/typelevel/cats?at=57bf284ac976227a1c501767

Related complications: I pretty much have a PR ready to go. One thing: Marking Xor and XorT methods with @deprecated causes the methods related to type class instances which are technically outside of the data types to throw up deprecation errors, so it doesn't look like we can actually put a @deprecated on it.

How do we want to proceed with this? Just put it in without @deprecated ?

@rossabaker
Copy link
Member

I'm in favor of deprecating what we can to guide people who haven't read the release notes. Because scalac is so rigid in its deprecation warnings and our own desire to use fatal warnings, "what we can" is sometimes nothing.

I'm opposed to bringing it back without deprecation. Once we're committed to breaking something, break it early, before more broken apps and libraries are written.

adelbertc added a commit to adelbertc/cats that referenced this issue Aug 25, 2016
@adelbertc
Copy link
Contributor Author

adelbertc commented Aug 25, 2016

PR that brings back just Xor and XorT with @deprecated: #1333

EDIT Nevermind, see below

@tpolecat
Copy link
Member

tpolecat commented Aug 25, 2016

When scalaz removed <| for 7.3.x it was not marked deprecated for the entire 7.2.x series, just for the micro releases after the decision was made. So in this spirit I think adding a deprecation warning for 0.7.1 and so on would be fine, assuming there will be a 0.7.1 release (there could be one just to add the warning I guess).

@adelbertc
Copy link
Contributor Author

I take back what I said about about the PR with @deprecated - it's getting more and more non-trivial.

  1. Marked data types as deprecated
  2. Marked all *InstancesN traits as deprecated since those refer to the deprecated data types and are outside of the data type
  3. Currently build is failing because the generated Doctest code is even more outside of this whole thing

Not sure how to get around (3) at the moment :|

@tpolecat
Copy link
Member

And for the record I'm perfectly happy telling -Xfatal-warnings people (i.e., me) they just need to turn it off while upgrading. Right now that's the only option Scala gives us and industry users are unlikely to be affected.

@kailuowang
Copy link
Contributor

resolved.

@adelbertc
Copy link
Contributor Author

Seeing activity on this scared me a bit, I thought we were bring Xor back ;)

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

7 participants