Determine supported IR versions automatically #3874
Conversation
gzm0
force-pushed
the
parse-versions
branch
2 times, most recently
from
787a386 to
f6d655f
Compare
| Set(binaryEmitted) | ||
| current = "1.0.0-SNAPSHOT", | ||
| /** Version of binary IR emitted by this version of Scala.js. */ | ||
| binaryEmitted = "1.0-SNAPSHOT" |
There was a problem hiding this comment.
These four lines should have 4 spaces of indentation.
There was a problem hiding this comment.
And the Scaladoc comments should be on the vals in VersionChecks. Actual arguments don't care about Scaladoc comments.
|
|
||
| private def parseFull(v: String): (Int, Int, Int, Option[String]) = { | ||
| val fullRE = """^([0-9]+)\.([0-9]+)\.([0-9]+)(-.*)?$""".r |
There was a problem hiding this comment.
Consider declaring that as a field of VersionChecks, similarly to binaryRE.
|
|
||
| private def parseBinary(v: String): (Int, Int, Option[String]) = { | ||
| val m = mustMatch(binaryRE, v) | ||
| (m.group(1).toInt, m.group(2).toInt, Option(m.group(3))) |
There was a problem hiding this comment.
| (m.group(1).toInt, m.group(2).toInt, Option(m.group(3))) | |
| (m.group(1).toInt, m.group(2).toInt, Option(m.group(3)).map(_.substring(1))) |
? So that the - does not belong to the result string.
| private def parseFull(v: String): (Int, Int, Int, Option[String]) = { | ||
| val fullRE = """^([0-9]+)\.([0-9]+)\.([0-9]+)(-.*)?$""".r | ||
| val m = mustMatch(fullRE, v) | ||
| (m.group(1).toInt, m.group(2).toInt, m.group(3).toInt, Option(m.group(4))) |
There was a problem hiding this comment.
| (m.group(1).toInt, m.group(2).toInt, m.group(3).toInt, Option(m.group(4))) | |
| (m.group(1).toInt, m.group(2).toInt, m.group(3).toInt, Option(m.group(4)).map(_.substring(1))) |
(see above)
|
|
||
| require(currentMinor >= binaryMinor, "minor(current) < minor(binaryEmitted)") | ||
|
|
||
| require(currentPreRelease.isEmpty || |
There was a problem hiding this comment.
Start the actual argument on the next line, since it spans multiple lines:
require(
currentPreRelease.isEmpty ||
...| binaryPreRelease == currentPreRelease, | ||
| "current is older than binaryEmitted through pre-release") | ||
|
|
||
| require(binaryPreRelease.isEmpty || |
There was a problem hiding this comment.
Same. In addition, add parens or braces around the && subexpression? I find it hard to read when we rely on the precedence of && over ||, especially in long expressions like this.
It looks like we need to add |
No description provided.