fix #155 by extracting version without suffix

[seigert]

As proposed by @julienrf I've changed semver version extraction method to allow suffixes after first three numbers.

Test are updated accordingly.

[julienrf]

Thank you @seigert for working on this!

I don’t think the fix is correct, please see my comment below.

[julienrf]

This is inconsistent with what versions does, see https://scastie.scala-lang.org/LQVPcpnLSuqRzz9z8J4jQA.

We should ignore the suffix of the current version only (as in https://scastie.scala-lang.org/miZPzsvfS7mrB6FGoyMVEQ).

So, we should have the following assertions:

isBreaking("1.0.0", "1.0.0-RC1")
isBreaking("1.0.1", "1.0.0-RC1")
isBreaking("1.1.0", "1.0.0-RC1")
isBreaking("2.0.0", "1.0.0-RC1")
isCompatible("1.0.0-RC1", "1.0.0-RC1")
isCompatible("1.1.0-RC1", "1.0.0")
isCompatible("1.2.1-SNAPSHOT", "1.2.0")

[seigert]

I've changed logic to ignore suffix only for current version with just one note: isCompatible("1.1.0-RC1", "1.0.0") => isCompatible("1.0.1-RC1", "1.0.0"), because 1.1.0 is allowed to be source incompatible.

This leads to question: what should we do with changes in suffix? Should RC2 vs RC1 or RC1 vs SNAPSHOT be assumed compatible or not?

[julienrf]

This leads to question: what should we do with changes in suffix? Should RC2 vs RC1 or RC1 vs SNAPSHOT be assumed compatible or not?

We should conform to the following checks:

isBreaking("1.0.0-RC2", "1.0.0-RC1")
isBreaking("1.1.0-RC2", "1.1.0-RC1")
// however:
isCompatible("1.0.1-RC2", "1.0.1-RC1")

The reason is that 1.0.1-RC1 and 1.0.1-RC2 are both candidates for patch releases, and patch releases cannot introduce source incompatibilities anyway.

[seigert]

isBreaking("1.0.0-RC2", "1.0.0-RC1")
isBreaking("1.1.0-RC2", "1.1.0-RC1")
// however:
isCompatible("1.0.1-RC2", "1.0.1-RC1")

I think I've implemented this correctly -- at least, all checks pass. :)

Also, I've reordered checks by (currentVersion, previousVersion, breaks > compatible).

[julienrf]

Thank you very much for the quick update!

I’ve left two minor comments, but otherwise it looks good to me!

[julienrf]

I wonder if this could be simplified to:

Suggested change

	def compatPatch =
	(currentSemVer.patch > 0 && samePatch) ||
	(currentSemVer.patch > previousSemVer.patch && previousSemVer.suffix.isEmpty)
	sameMajor && sameMinor && ((samePatch && sameSuffix) || compatPatch)
	def compatPatch =
	previousSemVer.suffix.isEmpty || previousSemVer.patch > 0
	sameMajor && sameMinor && compatPatch

(untested)

[seigert]

I've used your check, but we still need (samePatch && sameSuffix) to allow 1.0.0-RC1 be source compat to itself. :(

[julienrf]

Thank you @seigert

@sjrd, could you please trigger the CI and then merge the PR if it passes?

[sjrd]

[error] sbt-version-policy: Failed binary compatibility check against ch.epfl.scala:sbt-version-policy:2.1.2 (e:sbtVersion=1.0, e:scalaVersion=2.12)! Found 2 potential problems
[error]  * class sbtversionpolicy.DependencyCheckReport#SemVerVersion does not have a correspondent in other version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("sbtversionpolicy.DependencyCheckReport$SemVerVersion")
[error]  * object sbtversionpolicy.DependencyCheckReport#SemVerVersion does not have a correspondent in other version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("sbtversionpolicy.DependencyCheckReport$SemVerVersion$")
[error] Module sbt-version-policy:2.1.2+8-802b687c-SNAPSHOT is not source compatible with version 2.1.2. You have to relax your compatibility intention by changing the value of versionPolicyIntention.
[error] java.lang.RuntimeException: Failed binary compatibility check against ch.epfl.scala:sbt-version-policy:2.1.2 (e:sbtVersion=1.0, e:scalaVersion=2.12)! Found 2 potential problems
[error] (sbt-version-policy / versionPolicyMimaCheck) java.lang.RuntimeException: Failed binary compatibility check against ch.epfl.scala:sbt-version-policy:2.1.2 (e:sbtVersion=1.0, e:scalaVersion=2.12)! Found 2 potential problems

[seigert]

Should I add it to filters? Class is private and only method that uses it is private too.

[sjrd]

Yes, please add it to the filters with a comment that says it is private.

[seigert]

Yes, please add it to the filters with a comment that says it is private.

Done.

[julienrf]

Thank you @sjrd for merging the PR. It would be great to publish the fix by creating a new release on GitHub (version 2.1.3)