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

TS-28771 Remove version from agent JAR #189

Merged
merged 2 commits into from
Mar 1, 2022

Conversation

agoeb
Copy link
Contributor

@agoeb agoeb commented Nov 30, 2021

Addresses issue TS-28771

  • Changes are tested adequately
  • Agent's README.md updated in case of user-visible changes
  • CHANGELOG.md updated
  • Present new features in N&N
  • TGA Tutorial updated
  • TIA Tutorial updated

Please respect the vote of the Teamscale bot or flag irrelevant findings as tolerated or false positives. If you feel that the Teamscale config needs adjustment, please state so in a comment and discuss this with your reviewer.

Sorry, something went wrong.

Copy link
Contributor

@DreierF DreierF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on the fix!

Could we add a simple check to the github actions pipeline (as suggested by @karottenreibe) that ensures the version number is not contained in the zip as expected?

// Needed to make git properties work with Java 8, see https://github.com/n0mer/gradle-git-properties/issues/195
configurations.all {
resolutionStrategy {
force 'org.eclipse.jgit:org.eclipse.jgit:5.13.0.202109080827-r'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not have an effect on the buildscript classpath (see failing build).

Could guess that this solution is required.

classifier = null
// since this is used as an agent, we want it to always have the same name
// otherwise people have to adjust their -javaagent parameters after every
// update
archiveVersion = null
version = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move that version number reset out of the shadowJar block as it suggests that this is a property of the jar task, but actually this sets the version of the complete project.

@@ -54,12 +54,12 @@ shadowJar {
// Needed as a workaround for https://github.com/johnrengelman/shadow/issues/521
inputs.property("debug", project.hasProperty("debug"))

archiveBaseName = 'teamscale-jacoco-agent'
baseName = 'teamscale-jacoco-agent'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baseName is deprecated and will be removed in the next Gradle version. The documentation suggests to use archiveBaseName instead. Are we sure that this is needed to get it back working?

@DreierF DreierF force-pushed the master branch 2 times, most recently from 4ca10d7 to 5ae23ec Compare February 14, 2022 15:54
@DreierF DreierF force-pushed the cr/28771_remove_jar_version branch from 87df68c to 59ec699 Compare February 15, 2022 08:27
@stahlbauer stahlbauer merged commit fa27d33 into master Mar 1, 2022
@karottenreibe karottenreibe deleted the cr/28771_remove_jar_version branch March 28, 2022 09:23
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

3 participants