-
Notifications
You must be signed in to change notification settings - Fork 493
allow disabling of Google Container Registry credential checks #43
Conversation
…maven into jeremyweber-np-DM-34
`googleContainerRegistryEnabled` seems more maven idiomatic
} | ||
} catch (IOException ex) { | ||
getLog().info("ignoring exception while loading Google credentials", ex); | ||
if (googleContainerRegistryEnabled){ |
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.
Missing space in front of {
;)
getLog().info("ignoring exception while loading Google credentials", ex); | ||
} | ||
} else { | ||
getLog().info("Google Container Registry support is disabled"); |
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.
.debug()
?
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.
there is an info line already in googleContainerRegistryAuthSupplier when credentials are successfully loaded, so i wanted to have some parity
* can be explicitly disabled with this property if needed. | ||
*/ | ||
@Parameter(defaultValue = "true", property = "dockerfile.googleContainerRegistryEnabled") | ||
private boolean googleContainerRegistryEnabled = true; |
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.
No need to have both defaultValue = ...
and assign a default value
suppliers.add(0, googleSupplier); | ||
} | ||
} catch (IOException ex) { | ||
getLog().info("ignoring exception while loading Google credentials", ex); |
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.
Uppercase i
in ignoring
I released version 1.3.3 of the plugin a few minutes ago with this change in it. |
This is a continuation of #35; I added a commit addressing my own feedback to be able to merge and release this quicker.