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

Deprecate Either#right and Either#left projections #6682

Merged
merged 1 commit into from
Aug 8, 2018

Conversation

martijnhoekstra
Copy link
Contributor

Deprecates the Either projections again for 2.13.x

Originally deprecated in #5135

Deprecation made undone in #5433 which indicates

for two reasons:

  • to facilitate warning-free cross-compilation between Scala 2.11
    and 2.12
  • because it's not clear that .swap is a good replacement for .left

Either.right seems almost certain to be deprecated in 2.13.
Either.left's future is uncertain; see discussion (and links to
additional discussions) at #5135

Warning-free cross-compilation between 2.11 and 2.13 is probably not a big requirement, and not really a feasible aim in the first place.

.swap seems to have proven itself.

@scala-jenkins scala-jenkins added this to the 2.13.0-M5 milestone May 28, 2018
@julienrf
Copy link
Contributor

julienrf commented May 28, 2018

Using .right is quite common. It’s unfortunate to get a warning for that when you maintain a cross-compiling code base. Do you think we could provide a compatibility library (or extend scala-collection-compat to be scala-library-compat) that would make eitherAOrB.map(b => f(b)) compile on 2.12?

@martijnhoekstra
Copy link
Contributor Author

@julienrf no compat library is needed to compile map on 2.12 - you don't need .right on 2.12, just on 2.11, and this PR targets 2.13.x.

Cross-compilation of 2.11 and 2.13 without warnings sounds like a tall order, given the deprecations there already are in 2.13.x

That said, I have no objection in investigating whether adding extension methods to scala-collection-compat for Either so we can get (close to) feature parity is feasible.

@julienrf
Copy link
Contributor

Right, I meant 2.11, which is still widely supported by libraries.

@martijnhoekstra
Copy link
Contributor Author

@martijnhoekstra
Copy link
Contributor Author

But what is the story with source compat now? Deprecation requires an alternative that makes sure you can compile the same source with no warnings across 3 major versions?

@julienrf
Copy link
Contributor

Yes, something like you did (except that you should have left the collections in the scala-2.11_2.12 directory). I’m still not sure we need this though, I wanted to see if it was easy to provide.

With your addition to the compat library, Either would become right biased in 2.11 too, which would make it possible to have a warning free code base compatible with 2.11, 2.12 and 2.13.

@lrytz
Copy link
Member

lrytz commented May 30, 2018

Deprecation requires an alternative that makes sure you can compile the same source with no warnings across 3 major versions?

No, I don't think so. It's a best effort / cost-value thing. I'd say warnings for cross-compiling 2.11 - 2.13 is not unexpected.

Ordinary projects don't cross-compile, only libraries do, so most people don't have to deal with this question.

So LGTM!

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Let's get this show on the road.

@SethTisue SethTisue self-assigned this Aug 7, 2018
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Aug 7, 2018
@lrytz
Copy link
Member

lrytz commented Aug 8, 2018

@SethTisue merge?

@SethTisue
Copy link
Member

SethTisue commented Aug 8, 2018

I'm a little nervous that doing the deprecation will lead to us discovering that .swap isn't as well-liked as we think/hope, but there's still time to revert this before 2.13.0 if there's a lot of backlash.

@SethTisue SethTisue merged commit 0fd1b66 into scala:2.13.x Aug 8, 2018
@milessabin
Copy link
Contributor

This is what I came up with for shapeless.

@SethTisue SethTisue changed the title deprecate either projections Deprecate Either#right and Either#left projections Aug 22, 2018
def left = Either.LeftProjection(this)

/** Projects this `Either` as a `Right`.
*
* Because `Either` is right-biased, this method is not normally needed.
*/
@deprecated("Either is now right-biased", "2.13.0")
Copy link
Member

Choose a reason for hiding this comment

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

This message should explain more directly how users should fix their code.

@SethTisue
Copy link
Member

the deprecation of .left has been reverted by #8012

@SethTisue SethTisue removed the release-notes worth highlighting in next release notes label May 2, 2019
@som-snytt
Copy link
Contributor

The deprecation of right ought to have been reverted.

I have to change e.right.get to e.toOption.get because getOrElse requires specifying an alt path that doesn't happen.

There was a lot of talk on the various PRs about how there is no rush and the way forward isn't clear, which indicates doing nothing.

@SethTisue
Copy link
Member

should the deprecation message suggest .toOption.get? it doesn't seem so bad to me — it's certainly easier to understand than .right, which requires understanding a whole concept (projection) you might otherwise be able to avoid learning

@dwijnand
Copy link
Member

I agree. Using .toOption.get seems fine to me. But I do agree that "Either is now right-biased" could be more user-friendly.

@som-snytt
Copy link
Contributor

Either is now right-biased, but not in the political sense.

Either is now right-biased, like the rest of the world, as all lefties know.

rtyley added a commit to rtyley/scalatest that referenced this pull request Feb 13, 2020
* scala/scala#5135 - `Either` became right-biased
  in Scala 2.12
* scala/scala#6682 - `either.right` becomes a
  deprecated field with Scala 2.13

With Scala 2.13, it's no longer possible to test an `Either`'s right value
using ScalaTest's `EitherValues` without getting a deprecating warning -
ie writing:

```
either.right.value should be > 9
```

will give you this on compile:

```
method right in class Either is deprecated (since 2.13.0): Either is now
right-biased, use methods directly on Either
```

Given that `Either` is now right-biased, this change gives us the ability
to write:

```
either.value should be > 9
```
rtyley added a commit to rtyley/scalatest that referenced this pull request Feb 13, 2020
* scala/scala#5135 - `Either` became right-biased
  in Scala 2.12
* scala/scala#6682 - `either.right` becomes a
  deprecated field with Scala 2.13

With Scala 2.13, it's no longer possible to test an `Either`'s right value
using ScalaTest's `EitherValues` without getting a deprecating warning -
ie writing:

```
either.right.value should be > 9
```

will give you this on compile:

```
method right in class Either is deprecated (since 2.13.0): Either is now
right-biased, use methods directly on Either
```

Given that `Either` is now right-biased, this change gives us the ability
to write:

```
either.value should be > 9
```
aloiscochard pushed a commit to alephium/scala-common that referenced this pull request Aug 7, 2020
- `RightProjection` is deprecated, `toOption.get` is now recommended: scala/scala#6682
- varargs now works only for immutable Seq, so in `Network` we can't pass
  directly our `Array`: https://docs.scala-lang.org/overviews/core/collections-migration-213.html
- Regarding the `import Ordering.Double.IeeeOrdering` in `AVectorSpec`,
  have a look at: https://www.scala-lang.org/api/2.13.2/scala/math/Ordering$$Double$.html
aloiscochard pushed a commit to alephium/scala-common that referenced this pull request Sep 15, 2020
- `RightProjection` is deprecated, `toOption.get` is now recommended: scala/scala#6682
- varargs now works only for immutable Seq, so in `Network` we can't pass
  directly our `Array`: https://docs.scala-lang.org/overviews/core/collections-migration-213.html
- Regarding the `import Ordering.Double.IeeeOrdering` in `AVectorSpec`,
  have a look at: https://www.scala-lang.org/api/2.13.2/scala/math/Ordering$$Double$.html
aloiscochard pushed a commit to alephium/scala-common that referenced this pull request Sep 18, 2020
- `RightProjection` is deprecated, `toOption.get` is now recommended: scala/scala#6682
- varargs now works only for immutable Seq, so in `Network` we can't pass
  directly our `Array`: https://docs.scala-lang.org/overviews/core/collections-migration-213.html
- Regarding the `import Ordering.Double.IeeeOrdering` in `AVectorSpec`,
  have a look at: https://www.scala-lang.org/api/2.13.2/scala/math/Ordering$$Double$.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants