You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/08/03 14:58:16 UTC

[GitHub] [incubator-pekko] mdedetrich opened a new pull request, #532: Automatically handle MiMa for all versions

mdedetrich opened a new pull request, #532:
URL: https://github.com/apache/incubator-pekko/pull/532

   The primary goal of this PR is to automatically handle checking for MiMa while in the process removing the hardcoding of `latestPatchOf10` (which is currently incorrect since it should be `1` due to Pekko 1.0.1 being released but the fact that we forgot to increment this strengthens the argument for doing this automatically).
   
   The PR is designed to handle the following cases
   
   * Minor being bumped (i.e. when we just went from `1.0.x` to `1.1.x`) but no version yet released. For this case we compare to the entire previous minor release (i.e. `1.0.0` and `1.0.1` with the example of `1.1.x`)
   * Just after the first release of a minor (i.e. `1.0.0`) in which case we just compare with `1.0.0`
   * The standard case, i.e. with `1.0.1` released then we compare to all of the previous versions from the major series up until this one.
   
   The code is cleanly commented and its also easy to test, i.e. in `build.set` set
   
   ```sbt
   ThisBuild / version := "<VERSION>"
   ```
   
   to whatever version you want to test with and then just run `sbt mimaPreviousArtifacts`. This will give you a list of all of computed artifacts to check against.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] pjfanning commented on pull request #532: Automatically handle MiMa for all versions

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1664741546

   build failed because we have no 1.1.0-M0 jars to compare against.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] pjfanning commented on pull request #532: Automatically handle MiMa for all versions

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1664156760

   I think it be easier to explicitly name the version that we are diffing against. For me, both 1.0.x and main should diff against 1.0.0 release.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] mdedetrich commented on pull request #532: Automatically handle MiMa for all versions

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1664304992

   Okay PR is ready to review. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] mdedetrich commented on pull request #532: Automatically handle MiMa for all versions

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1664172927

   > I think it be easier to explicitly name the version that we are diffing against. 
   
   The problem is that this is brittle, if we forget to bump the version (as we just did) then we can introduce bincompat problems. The logic of what we want to do is perfectly easy to automate
   
   > For me, both 1.0.x and main should diff against 1.0.0 release.
   
   Yes and this is retained


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] He-Pin commented on pull request #532: Automatically handle MiMa for all versions

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1704301633

   @mdedetrich There is no milestone release


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] Automatically handle MiMa for all versions [incubator-pekko]

Posted by "gmethvin (via GitHub)" <gi...@apache.org>.
gmethvin commented on code in PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#discussion_r1468685903


##########
project/MiMa.scala:
##########
@@ -29,35 +27,36 @@ object MiMa extends AutoPlugin {
 
   override val projectSettings = Seq(
     mimaReportSignatureProblems := true,
-    mimaPreviousArtifacts := pekkoPreviousArtifacts(name.value, organization.value),
-    checkMimaFilterDirectories := checkFilterDirectories(baseDirectory.value))
-
-  def checkFilterDirectories(moduleRoot: File): Unit = {
-    val nextVersionFilterDir =
-      moduleRoot / "src" / "main" / "mima-filters" / s"1.0.${latestPatchOf10 + 1}.backwards.excludes"
-    if (nextVersionFilterDir.exists()) {
-      throw new IllegalArgumentException(s"Incorrect mima filter directory exists: '$nextVersionFilterDir' " +
-        s"should be with number from current release '${moduleRoot / "src" / "main" / "mima-filters" / s"1.0.$latestPatchOf10.backwards.excludes"}")
-    }
-  }
-
-  def pekkoPreviousArtifacts(
-      projectName: String,
-      organization: String): Set[sbt.ModuleID] = {
-    val versions: Seq[String] = {
-      val firstPatchOf10 = 0
-
-      val pekko10Previous = expandVersions(1, 0, 0 to latestPatchOf10)
-
-      pekko10Previous
+    mimaPreviousArtifacts := {
+      previousStableVersion.value match {
+        case Some(previousStableVersion) =>
+          val Some((currentMajor, currentMinor)) = CrossVersion.partialVersion(version.value)
+          val Some((previousStableMajor, _)) = CrossVersion.partialVersion(previousStableVersion)
+          // If we bump up the major then lets assume this intentionally breaks binary compatibility
+          // so lets not check any artifacts
+          // See https://github.com/sbt/sbt-dynver/issues/70#issuecomment-458620722
+          if (currentMajor == previousStableMajor + 1 && currentMinor == 0)
+            Set.empty
+          else
+            Set(organization.value %% moduleName.value % previousStableVersion)
+        case None => Set.empty
+      }
+    },
+    checkMimaFilterDirectories := checkFilterDirectories(baseDirectory.value, version.value))
+
+  def checkFilterDirectories(moduleRoot: File, version: String): Unit = {
+    val strippedPatchRegex = """(\d+)(.*)""".r

Review Comment:
   This regex doesn't make sense if we're matching against the whole version—if you have a version of `1.1.0` then you're extracting `.1.0` as the patch version which doesn't parse as an int. Did you want something like `\d+[.]\d+[.](\d+).*`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] mdedetrich commented on pull request #532: Automatically handle MiMa for all versions

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1667527994

   > build failed because we have no 1.1.0-M0 jars to compare against.
   
   So this solution would be much more cleanly solved by just publishing a milestone artifact. Tagging something as milestone but not publishing its artifact is all things considered not idiomatic. We can discuss whether it makes sense to bundled a `M0` release together with the last patch version of the previous minor since technically speaking the source will be the same, its just that the versioning is changing since we are creating a branch.
   
   Otherwise the only other way to solve this problem is to hack some kind of workaround and/or not use milestone tags and deal with the fact that until the first release is made for a new minor the snapshots will not line up.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] He-Pin commented on pull request #532: Automatically handle MiMa for all versions

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #532:
URL: https://github.com/apache/incubator-pekko/pull/532#issuecomment-1704297293

   Let me close and reopen to trigger a rebuild.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


[GitHub] [incubator-pekko] He-Pin closed pull request #532: Automatically handle MiMa for all versions

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin closed pull request #532: Automatically handle MiMa for all versions
URL: https://github.com/apache/incubator-pekko/pull/532


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org