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

improve when signing tasks run, consider all targets for signing #30

Merged
merged 2 commits into from
Oct 9, 2018
Merged

improve when signing tasks run, consider all targets for signing #30

merged 2 commits into from
Oct 9, 2018

Conversation

gabrielittner
Copy link
Collaborator

  • signing configuration isn't part of afterEvaluate anymore which makes manually overriding it easier
  • required (= task fails when signing isn't set up on the local machine) only considers if the task is a snapshot
  • signing will only be run when also running an Upload task (not during things like assemble)
  • the previous point will now consider all targets, before only uploadArchives was considered
  • added info level logs to make debugging issues easier

fixes #29

@gabrielittner
Copy link
Collaborator Author

One downside of the new onlyIf implementation is that you can't run signing explicitly. ./gradlew signArchives will skip the task because none of the upload tasks are in the task graph.

@codecov
Copy link

codecov bot commented Oct 8, 2018

Codecov Report

Merging #30 into master will decrease coverage by 5.9%.
The diff coverage is 42.1%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #30      +/-   ##
============================================
- Coverage        90%   84.09%   -5.91%     
- Complexity       13       15       +2     
============================================
  Files             3        3              
  Lines           120      132      +12     
  Branches         14       14              
============================================
+ Hits            108      111       +3     
- Misses            5       13       +8     
- Partials          7        8       +1
Impacted Files Coverage Δ Complexity Δ
...com/vanniktech/maven/publish/MavenPublishTarget.kt 70% <33.33%> (-15.72%) 5 <2> (+2)
...vanniktech/maven/publish/MavenPublishPlugin.groovy 87.61% <43.75%> (-6.14%) 6 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b908e6...cb41397. Read the comment docs.

)
) {

val taskName get(): String {
Copy link
Owner

Choose a reason for hiding this comment

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

Are there tests for this given that the API is public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add them. I wanted to make it internal, but Groovy didn't like that for some reason.

@vanniktech
Copy link
Owner

One downside of the new onlyIf implementation is that you can't run signing explicitly. ./gradlew signArchives will skip the task because none of the upload tasks are in the task graph.

Totally fine.

@codecov-io
Copy link

codecov-io commented Oct 9, 2018

Codecov Report

Merging #30 into master will decrease coverage by 4.39%.
The diff coverage is 52.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master     #30     +/-   ##
==========================================
- Coverage        90%   85.6%   -4.4%     
- Complexity       13      16      +3     
==========================================
  Files             3       3             
  Lines           120     132     +12     
  Branches         14      14             
==========================================
+ Hits            108     113      +5     
- Misses            5      12      +7     
  Partials          7       7
Impacted Files Coverage Δ Complexity Δ
...com/vanniktech/maven/publish/MavenPublishTarget.kt 90% <100%> (+4.28%) 6 <3> (+3) ⬆️
...vanniktech/maven/publish/MavenPublishPlugin.groovy 87.61% <43.75%> (-6.14%) 6 <0> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b908e6...4d8bf93. Read the comment docs.

@gabrielittner
Copy link
Collaborator Author

Should be good now.

@gabrielittner gabrielittner merged commit 7bed96e into vanniktech:master Oct 9, 2018
@gabrielittner gabrielittner deleted the signing branch October 9, 2018 14:18
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.

Signing required not working as intended
3 participants