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

tlVersionIntroduced as a function of scalaVersion #24

Closed
rossabaker opened this issue Dec 30, 2021 · 7 comments · Fixed by #25
Closed

tlVersionIntroduced as a function of scalaVersion #24

rossabaker opened this issue Dec 30, 2021 · 7 comments · Fixed by #25

Comments

@rossabaker
Copy link
Member

Modules get introduced in different versions for different crossbuilds. sbt-spiewak takes a function of Scala versions to version introduced:

    versionIntroduced := Map(
      "3.0.0-RC1" -> "1.0.0",
      "3.0.0-RC2" -> "1.0.1",
      "3.0.0-RC3" -> "1.1.3"
    ),

I'm not sure how we express that in tlVersionIntroduced.

@armanbilge
Copy link
Member

I was thinking like:

.settings(
  tlVersionIntroduced := {
    if (tlIsScala3.value) Some("1.1.4") else None
  }
)

But, maybe not for everyone.

BTW, the mdoc build has a crossSetting helper to make defining version-specific settings even easier:

  scalacOptions ++= crossSetting(
    scalaVersion.value,
    if2 = List("-target:jvm-1.8", "-Yrangepos", "-deprecation"),
    if212 = List("-Xexperimental"),
    if211 = List("-Xexperimental"),
    if3 = List("-language:implicitConversions", "-Ximport-suggestion-timeout", "0")
  )

@armanbilge
Copy link
Member

Yeah, the Map thing seems good actually. Should have done that, let me fix it.

@rossabaker
Copy link
Member Author

I don't know without looking whether sbt-spiewak is a function or a map. Maybe a function is more power than is needed. I've always provided a map.

@armanbilge
Copy link
Member

armanbilge commented Dec 30, 2021

Yeah, I'm working on this now and I remembered why I don't like it: Scala version parsing from String and all its caveats, e.g. what if a user puts in "3.0" and "3.1" or just "3". I'd really rather just have something like this:

case class TLVersionIntroduced(
  scala212: Option[String] = None,
  scala213: Option[String] = None,
  scala3: Option[String] = None,
)

Or a solution like the crossSetting thing I posted above. Is there any reason we need to support Scala 3 RCs and stuff?

Edit: spiewak just expects binary version, so it would be 2.13 or just 3. I guess that works.

@rossabaker
Copy link
Member Author

case-insensitive needed the RCs because it was checking old versions, but we could have just turned it off for Scala 3. I removed it entirely in typelevel/case-insensitive#208.

I don't like the case class, because that bundles binary compatibility of this plugin with the set of Scala versions supported today. crossSetting looks dubious to me for a similar reason, though I don't know what it did under the hood. I could be happy with pattern matching on scalaBinaryVersion.value as long as it recalculates itself when we switch Scala versions.

@armanbilge
Copy link
Member

Oh, here's one more layer of complexity: now that we are checking MiMa for Scala.js and native as well, we also need to consider that axis, since these crosses were introduced later for some projects/modules. I guess that can be handled by defining a separate tlVersionIntroduced in .jvmSettings and .jsSettings etc., but there might be some duplication between those...

@rossabaker
Copy link
Member Author

After questioning life choices that brought me to that point, I'd do separate tlVersionIntroduced settings.

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

Successfully merging a pull request may close this issue.

2 participants