Suppress "no changes" output by default #92
Conversation
| implementation(libs.truth) | ||
| } | ||
|
|
||
| targets { | ||
| all { | ||
| configureEach { |
There was a problem hiding this comment.
This is a small opportunistic improvement as all eagerly configures targets and configureEach is lazy
| @@ -86,19 +96,24 @@ internal abstract class DependencyGuardListTask : DefaultTask() { | |||
| when (diffResult) { | |||
| is DependencyListDiffResult.DiffPerformed.HasDiff -> { | |||
| // Print to console in color | |||
| println(diffResult.createDiffMessage(withColor = true)) | |||
| logger.error(diffResult.createDiffMessage(withColor = true)) | |||
There was a problem hiding this comment.
I'm still understanding the Gradle Logging https://docs.gradle.org/current/userguide/logging.html
Why is this considered an error? Because of the priority that we want to have the user see this? Based on the docs QUIET seems like the right level as it is an Important information messages?
There was a problem hiding this comment.
error is always displayed to the user, and some folks may instrument their builds to capture and save/send/etc error level logs. Would it not be an error?
There was a problem hiding this comment.
A detected difference would be an error from a task result perspective.
So, error is good. Thanks 👍
|
|
||
| // Add to exception message without color | ||
| exceptionMessage.appendLine(diffResult.createDiffMessage(withColor = false)) | ||
| } | ||
|
|
||
| is DependencyListDiffResult.DiffPerformed.NoDiff -> { | ||
| // Print no diff message | ||
| println(diffResult.noDiffMessage) |
There was a problem hiding this comment.
Thanks for getting rid of these println statements. This is the artifact of a developer that didn't know how to write Gradle plugins, and println "worked just fine" before. This is much nicer, thanks!
There was a problem hiding this comment.
I left some random comments, but here is my overall review:
@ZacSweers - Big question. Does anyone ever want verbose logging? Was that a feature, or was it just something that was there?
I am a fan of log levels and being able to use those appropriately. I think I'd prefer less configuration options for the plugin. I am not seeing value in the messages saying "no changes found".
| if (logVerbosely.get()) { | ||
| logger.lifecycle(message) | ||
| } else { |
There was a problem hiding this comment.
Can we just delete these lines, and then get rid of the verbose option?
There was a problem hiding this comment.
I'm fine with that too, was just trying to leave existing functionality possible if it was wanted
There was a problem hiding this comment.
Appreciate it. Because it is just this case, I personally will not miss it. The goal of the dependencyGuard task is to succeed if no diffs, so there shouldn't need to be this extra logging.
If someone has a different option, an issue could be created for discussion, but this makes sense here IMO.
|
Sorry, I merged #89 because it was ready, so it'll require a re-base, but I know you were going to make updates/changes already, so that's why I merged that one now. 🙇 |
|
Finally updated! Sorry for the delay, got a bit tied up with kotlinconf and other stuff. I've updated it to also remove the logVerbosely option and instead just log that to logger.debug. So, it's still there if people need to debug something, but not noisily there in lifecycle. |
Opportunistically updated to the latest Gradle 7.x version + applied fixes needed from release notes.
Resolves #82. This also makes the "no changes detected" output off by default.