Clean build output for regular builds #698
Conversation
- very first draft
/** | ||
* If the exception should be thrown if the release is not needed. | ||
*/ | ||
public boolean isExplosive() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this one is public api, do we have to keep it and mark it deprecated? or can we remove it an create a 2.1.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We keep it and deprecate it. Can you create a static utility method like "deprecated(String message)" so that when we ship a new major version, we can wrap up all deprecations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea, let's do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice progress! Please remember to update the documentation and the templates (initShipkit task).
I love the idea of using StopExecutionException!
/** | ||
* If the exception should be thrown if the release is not needed. | ||
*/ | ||
public boolean isExplosive() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We keep it and deprecate it. Can you create a static utility method like "deprecated(String message)" so that when we ship a new major version, we can wrap up all deprecations?
"Checking if release is needed", asList("./gradlew", ReleaseNeededPlugin.ASSERT_RELEASE_NEEDED_TASK), ExecCommandFactory.stopExecution())); | ||
"Checking if release is needed", asList("./gradlew", ReleaseNeededPlugin.RELEASE_NEEDED), execResult -> { | ||
if (new File(project.getBuildDir(), ReleaseNeeded.RELEASE_NEEDED_FILENAME).exists()) { | ||
throw new StopExecutionException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really cunning idea. Nice!
LOG.lifecycle(message); | ||
File releaseNeededFile = new File(task.getProject().getBuildDir(), RELEASE_NEEDED_FILENAME); | ||
|
||
if (releaseNeededFile.exists()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! We need to delete the file.
Can you make it the first thing that happens in this task?
} | ||
|
||
if (releaseNeeded) { | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use IOUtil.writeFile() and save 8 lines of code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 , thx for this hint!
@@ -28,11 +33,24 @@ boolean releaseNeeded(ReleaseNeededTask task, EnvVariables envVariables) { | |||
boolean releaseNeeded = releaseNeed.needed; | |||
String message = releaseNeed.explanation; | |||
|
|||
if (!releaseNeeded && task.isExplosive()) { | |||
throw new GradleException(message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For backwards compatibility, please keep the behavior.
//Task that throws an exception when release is not needed is very useful for CI workflows | ||
//Travis CI job will stop executing further commands if assertReleaseNeeded fails. | ||
//See the example projects how we have set up the 'assertReleaseNeeded' task in CI pipeline. | ||
releaseNeededTask(project, ASSERT_RELEASE_NEEDED_TASK, conf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For backwards compatibility, please keep the task. I suggest to emit deprecated message when the task is executed, including information how to fix the deprecation.
@mockitoguy thx for your feedback. |
I'll complete the review today! Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added changes on top of your PR and we're ready. Check them out and push at will!
- stop execution if file is not present
I just did some local testing using shipkit-example project:
|
a very first draft for #664 in order to get early feedback.