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 CrossVersion.Disabled (was make CrossVersion.Disabled stable) #316

Merged
merged 2 commits into from
Aug 22, 2019

Conversation

eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented Aug 22, 2019

migration note

CrossVersion.Disabled did not work as expected in sbt 1.0.x ~ 1.2.x.

We recommend using CrossVersion.disabled instead. For pattern matching, you can either check equality with CrossVersion.disabled or type match with case _: sbt.librarymanagement.Disabled.

original PR

Fixes sbt/sbt#4975
Fixes sbt/sbt#4977

This makes CrossVersion.Disabled a stable identifier by reverting final def back to final val.

This is to fix Scala.JS build

[error] ScalaJSCrossVersion.scala:34:23: stable identifier required, but sbt.`package`.CrossVersion.Disabled found.
[error]     case CrossVersion.Disabled =>
[error]                       ^
[error] one error found
[error] (Compile / compileIncremental) Compilation failed

notes

Fixes sbt/sbt#4975

This makes `CrossVersion.Disabled` a stable identifier by reverting `final def` back to `final val`.

This is to fix Scala.JS build

```
[error] ScalaJSCrossVersion.scala:34:23: stable identifier required, but sbt.`package`.CrossVersion.Disabled found.
[error]     case CrossVersion.Disabled =>
[error]                       ^
[error] one error found
[error] (Compile / compileIncremental) Compilation failed
```

### notes

- sbt#121 added `final val Disabled = sbt.librarymanagement.Disabled` but it was just a companion object
- sbt#280 actually made it `final val Disabled = sbt.librarymanagement.Disabled()`, but this broke Cat's build that was calling `CrossVersion.Disabled()`
- sbt#290 changed to `final def Disabled = sbt.librarymanagement.Disabled` and `object Disabled extends sbt.librarymanagement.Disabled`
- This changes back to `final val Disabled = sbt.librarymanagement.Disabled` (but because we changed the companion object in sbt#290 that's ok)
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.

Ek. What a journey my mistake has been... sorry.

LGTM, thanks for preserving my change.

@eed3si9n
Copy link
Member Author

Given that CrossVersion.Disabled is used in the wild, maybe I should also retroactively create a bug and deprecate upper case Disabled in favor of CrossVersion.disabled for sbt 1.2.x users.

@eed3si9n eed3si9n merged commit 705f64c into sbt:develop Aug 22, 2019
@eed3si9n eed3si9n deleted the wip/disabled branch August 22, 2019 17:48
eed3si9n added a commit to eed3si9n/librarymanagement that referenced this pull request Aug 28, 2019
sbt#316 started returning Disabled companion object for `apply()` instead of creating a new instance.
This leaked into JSON as it uses runtime types for union. This works around that by providing a hand-crafted format.

Ref sbt/sbt#4997
@eed3si9n eed3si9n changed the title Make CrossVersion.Disabled stable Deprecate CrossVersion.Disabled (was make CrossVersion.Disabled stable) Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants