Skip to content

Makes shipkit / nexusPublish a convention plugin for root #3533

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

Merged
merged 3 commits into from
Dec 22, 2024

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Dec 9, 2024

Sibling of #3532

Following on the trend to follow best practice, the shipkit.gradle script is now a convention plugin for the root project.

One more step to help on #3474

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should mention Fixes #<issue number> if relevant

@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.55%. Comparing base (68c4285) to head (22c4243).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #3533   +/-   ##
=========================================
  Coverage     85.55%   85.55%           
  Complexity     2955     2955           
=========================================
  Files           341      341           
  Lines          8995     8995           
  Branches       1119     1119           
=========================================
  Hits           7696     7696           
  Misses         1009     1009           
  Partials        290      290           

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

@bric3 bric3 marked this pull request as ready for review December 9, 2024 00:30
Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

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

Some nits, the rest LGTM

@@ -11,6 +11,9 @@ gradleplugin-animal-sniffer = "1.7.2"
gradleplugin-aQute-bnd = "7.0.0"
gradleplugin-errorprone = "4.1.0"
gradleplugin-spotless = "6.25.0"
gradleplugin-nexusPublish = "2.0.0-rc-1"
gradleplugin-shipkit-changelog = "2.0.1"
gradleplugin-shipkit-autoVersion = "2.1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: since there is only 1 plugin that uses this version, I think it is duplicate to put it here as well. Therefore, if there is only one artifact with a particular version reference, I would prefer to use version instead of version.ref

tasks {
generateChangelog {
githubToken = providers.environmentVariable("GITHUB_TOKEN").get()
previousRevision = project.ext["shipkit-auto-version.previous-tag"].toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

In a follow-up PR, let's refactor this so that we also use an environment variable for this.

dependsOn(generateChangelog)
githubToken = providers.environmentVariable("GITHUB_TOKEN").get()
newTagRevision = providers.environmentVariable("GITHUB_SHA").get()
repository = generateChangelog.get().repository
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about defining constants values instead of referencing separate tasks here. That removes the need for eager configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed that came to mind. I avoided this here to make a 1:1 translation. But this would be better in my opinion.

@bric3 bric3 force-pushed the shipkit-convention-plugin branch from e50e5ae to e7d8627 Compare December 21, 2024 23:28
@bric3 bric3 force-pushed the shipkit-convention-plugin branch from e7d8627 to 22c4243 Compare December 21, 2024 23:34
@bric3 bric3 mentioned this pull request Dec 21, 2024
2 tasks
@TimvdLippe TimvdLippe merged commit 2c184c9 into main Dec 22, 2024
18 checks passed
@TimvdLippe TimvdLippe deleted the shipkit-convention-plugin branch December 22, 2024 07:08
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.

3 participants