Skip to content
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

remove VersionScmPosition as it is a duplicate of ScmPosition #686

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

rzabini
Copy link
Contributor

@rzabini rzabini commented Dec 3, 2023

Using scmVersion.scmPosition property to add information to a jar manifest, I noticed that the class VersionScmPosition was missing the isClean property.
I though about adding another field to it, but then I saw that this class is basically a replica of ScmPosition (which has the isClean field), so maybe it is cleaner to remove altogether the class, and use ScmPosition instead.
What do you think?

@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (9eee1c8) 60.84% compared to head (49cee8e) 61.00%.

Files Patch % Lines
...ch/build/axion/release/domain/VersionConfig.groovy 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #686      +/-   ##
============================================
+ Coverage     60.84%   61.00%   +0.15%     
  Complexity      398      398              
============================================
  Files            82       81       -1     
  Lines          1558     1554       -4     
  Branches        147      147              
============================================
  Hits            948      948              
+ Misses          539      535       -4     
  Partials         71       71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bgalek
Copy link
Member

bgalek commented Dec 3, 2023

Hi, seems like a good idea! Care for a PR? :)

@bgalek
Copy link
Member

bgalek commented Dec 4, 2023

LGTM :)

@bgalek bgalek merged commit 5472530 into allegro:main Dec 4, 2023
1 check passed
@gabrieljones
Copy link
Contributor

gabrieljones commented Dec 7, 2023

please add this back and deprecate

java.lang.NoSuchMethodError: 'pl.allegro.tech.build.axion.release.domain.VersionScmPosition pl.allegro.tech.build.axion.release.domain.VersionConfig.getScmPosition()'
object StandardManifest {
  fun apply(jar: Jar, scmVersion: VersionConfig?, project: Project) {
    jar.manifest.attributes.apply {
      put("Build-By", System.getProperty("user.name"))
      put("Build-JDK", org.gradle.internal.jvm.Jvm.current())
      put("Build-Revision", scmVersion?.scmPosition?.revision ?: "axion-release not present")
      put("Specification-Title", project.name)
      put("Specification-Version", "${project.version}-${scmVersion?.scmPosition?.shortRevision}")
      put("Implementation-Title", project.name)
      put("Implementation-Version", "${project.version}-${scmVersion?.scmPosition?.shortRevision}")
      put("Implementation-Vendor-Id", project.group)
    }
  }
}

@bgalek
Copy link
Member

bgalek commented Dec 7, 2023

hi @gabrieljones, which fields are you missing? we can add them to ScmPosition class

@gabrieljones
Copy link
Contributor

The return type change on getScmPosition is causing the exception. I created a pre compiled convention plugin that calls getScmPosition. Downstream users of this plugin that have upgraded their axion release version are seeing the NoSuchMethodError.

@gabrieljones
Copy link
Contributor

If I am the only one experiencing this then we can close this as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants