Fix #170 When bintray.key is null the bintrayUpload task silently fails #186
Conversation
| errors.add("key is not configured") | ||
| } | ||
| if(!errors.isEmpty()) { | ||
| errors.join(" and ") |
There was a problem hiding this comment.
It is used, see should throw an exception when user and api key is not configured test.
There was a problem hiding this comment.
Ahhh ok in next line I have this same.
| } | ||
| if(!errors.isEmpty()) { | ||
| errors.join(" and ") | ||
| def message = "Failing task '${this.project.name}:bintrayUpload' because ${errors.join(" and ")}" |
There was a problem hiding this comment.
It is not desirable to duplicate the task name here. Instead of ${this.project.name}:bintrayUpload we can just use $path. Every task has 'path' property which I think is exactly what we need here :)
There was a problem hiding this comment.
I just copied and modified a message from line 177. I can fix that if needed.
| throw new GradleException(message) | ||
| } | ||
| } | ||
|
|
||
| boolean shouldSkip() { |
There was a problem hiding this comment.
It's used in getAllBintrayUploadTasks()
| logger.info("Skipping task '{}:bintrayUpload' because user or apiKey is null.", this.project.name); | ||
| return | ||
| } | ||
| assertUploadConfiguration() |
There was a problem hiding this comment.
Warning to bintray plugin owners: this change is backwards incompatible. This change makes sense from the standpoint of fast feedback and the principle of least surprise (silently skipping is bad). If you choose to roll out incompatible change please document it well and apply semver principles (bump major version).
There was a problem hiding this comment.
If JFrog does not want to make a missing API key or user fail the build, then at least log a warning instead of silently failing. Just changing logger.info to logger.warn would save a lot of people a lot of time as warnings would be logged by Gradle by default.
|
Thank you @mstachniuk for taking the initiative to fix the problem! |
|
@eyalbe4 Could you look on this PR please? |
|
Sure @mstachniuk. Thank you for this. It is great to see the collaboration and team work. |
|
@mstachniuk and @szczepiq, |
|
Any update here? I got burned by this hard today. |
|
I think that this issue can be closed, because the plugin's behaviour has been modified in the lastest releases and this issue should be resolved now. |
No description provided.