SI-6963 Add version to -Xmigration #1998
Conversation
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1661/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2392/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1661/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2394/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2396/ |
|
jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2394/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2396/ |
| * final, release candidate, milestone, and development builds. The build argument is used | ||
| * to segregate builds | ||
| */ | ||
| case class ActualScalaVersion(major: Int, minor: Int, rev: Int, build: ScalaBuild) extends ScalaVersion { |
There was a problem hiding this comment.
I find "actual" to be highly unrevealing as to what something is - what is "actual" is different for different people. Ideas I would like better:
- PreciseScalaVersion
- UniversalScalaVersion
- BuiltScalaVersion
I can come up with more if we don't like any of those.
There was a problem hiding this comment.
SpecificScalaVersion?
There was a problem hiding this comment.
Yeah, that came to mind too. I had some reservation about it but I can't think of it now. It's good with me.
|
So what was the decision on how to handle external code? How can people who use their own version scheme for their libraries benefit from I think it would be a far cleaner approach to make |
|
@soc Supporting 3rd party libraries means figuring out version schemas. If you have a proposal I'm all ears. Making migration work with and without an argument leads to an ambiguity. scalac -migration X. What is X? Is it a file or a version argument? I could try to parse it as a version argument and if that fails leave it on the queue to be parsed as a file argument. But then innocent typos turn into bizarre errors. Further, and more subtly, there's nothing that says a scala file can't be called "2" right now but making migration slurp up anything that it can parse as a version would lead to a different error. Note, I'm not "adding another option." I'm removing an option and putting in a replacement. It's just that I can't remove immediately, I have to deprecate. |
No, but that's why exactly the changes proposed here have been abandoned more than a year ago.
If Look here: #49 |
|
Idea: Let's use the information from the MANIFEST.MF file. This will work for 95% of the people who don't compile every third-party library from scratch. There are multiple ways to handle the remaining 5%:
Opinions? |
|
@soc, I'm making the changes now to go with -Xmigration:version where version is optional. MANIFEST.MF tells you what the current version of the library is, it doesn't tell you what version the user wrote the code for. To have any hope of working the migration annotation would look like "@migration(artifactid, versionChangedIn, info about the change)" and the user would type something like scalac -Xmigration:artifactId=versionWrittenFor. And we'd have to insist that version numbers be in number[.number]* so we could parse and compare them. That change would have to wait for 2.11 because it breaks the migration annotation's compatibility. In the meantime I can make -Xmigration substantially more useful without precluding that design later by going with -Xmigration:version in 2.10. |
<
8000
a href="/scala/scala/pull/1998/commits/1de399d3c655807465c6369f77d08e57743e7eaa" class="Link--secondary">1de399d
Adds an optional version parameter to the -Xmigration compiler setting. Doing -Xmigration without version number behaves as it used to by dumping every possible migration warning. This commit adds a ScalaVersion class (plus ancillary stuff), and a ScalaVersionSetting.
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1704/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2439/ |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1708/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2443/ |
|
Ok, thanks! Looks a lot better that way. Considering the MANIFEST.MF stuff: I don't think it has to be that complicated. The version in an migration annotation is compared to the version in the MANIFEST file of the jar file to which the class belongs. If that version is in the not so far future, a warning is emitted. I think that way we would have a sane default, which would also make the version stuff less important. But let's go forward with what we have now. :-) |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2443/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1704/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1708/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2439/ |
|
ping @adriaanm for review |
|
LGTM, noice! |
SI-6963 Add version to -Xmigration
Adds an optional version parameter to the -Xmigration compiler setting.
Doing -Xmigration without version number behaves as it used to by
dumping every possible migration warning.
This commit adds a ScalaVersion class (plus ancillary stuff), and a
ScalaVersionSetting.
Since @adriaanm convinced me to add a version number, I'll give this to him for review.