-
Notifications
You must be signed in to change notification settings - Fork 81
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
Add Scala 3.1.0 support #568
Conversation
1582a87
to
d4e4b8c
Compare
build.sbt
Outdated
@@ -65,6 +67,15 @@ def crossSetting[A]( | |||
case _ => Nil | |||
} | |||
|
|||
def publishCrossVersion(scalaVersion: String): CrossVersion = { | |||
// similar to `CrossVersion.binary` | |||
// but for scala3 write both major and minor instead of just "3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, can you elaborate on why we need this? I'm getting lost in the whole 3.1/3.0 situation so would appreciate some extra information.
I thought that we can publish mdoc with whatever Scala 3 version we want, and if it's a minor
below the end user's version, they won't have to upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this publishing scheme(having only _3
in postfix) mdoc might be run on on the latest version with incompatible scala3-library in classpath that would lead to non clear errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make sure I'm following, this will publish _3.1
and _3.0
artifacts? If so, and maybe a silly question, but then how would a user of mdoc include this in their build? Since it's not the default _3
that the build tool would look for? Or am I totally missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ckipp01 it's handled by MdocPlugin
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that assumes that someone is using it through the plugin. What if I'm just using mdoc as a library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I'm asking really is because mdoc seems to be the one setting precedent here - i.e. I'm unaware of any libraries using the major.minor versioning strategy.
The errors that I've seen were actually down to a weird coursier's resolution of the artifacts, I put my findings here:
I think before we change the versioning scheme to something that nobody else is doing (or even requiring to do), we need to make sure this is the only way. And in that case I'd bring folks from Dotty team to confirm.
Do you have somewhere else I can look for information on how you arrived at this solution? Not sure if it's #525 you're working off of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am indeed one of those users of mdoc without plugin, so this affects me directly in several internal projects :)
But on another note, I still think we should investigate alternatives or indeed the root cause.
This goes against the way Scala 3 artifacts are supposed to be published, and while I understand blocking metals release is unfortunate, this will produce artifacts which the rest of the ecosystem will not know how to treat, as Scala 3's binary artifact version is supposed to be _3
and that's it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keynmol Ok, I will revert this thing and do just upgrade to 3.1.0. Tested it on metals - works well with 3.0.2 in classpath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 awesome
Notes: scala3 artifacts are published with_$major.$minor
postfixAfter a discussion we decided just to upgrade scala 3 version