-
Notifications
You must be signed in to change notification settings - Fork 133
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
Enhance PackageBuildOutputs sample #100
Enhance PackageBuildOutputs sample #100
Conversation
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.
See my comment.
* -artifactory, | ||
* --artifactoryPropertiesFile <propertyFile> Absolute path of a property file | ||
* containing application specific |
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.
I'm wrong, or you are breaking the current behavior?
Can we keep the -prop,--propertyFile <propertyFile>
with a deprecated notice?
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 now implemented. Resolved
// read properties file | ||
if (opts.properties){ | ||
def propertiesFile = new File(opts.properties) | ||
if (propertiesFile.exists()){ | ||
propertiesFile.withInputStream { props.load(it) } | ||
} | ||
} | ||
|
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.
Could you add an else block to read the default property file you shipped in your PR ? (to avoid the assertion later and add backward compatibility)
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.
default packageBuildOutputs.properties
file is tried to be read, if the --properties
option was not passed.
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.
I have just left a comment.
Look good.
* externalize copymodes and allow packaging of additional build logs * Update readme * implements IBM#97 to mask artifactory password * Cli option -prop marked as deprecated Signed-off-by: John Nemec <john.nemec@ibm.com>
* externalize copymodes and allow packaging of additional build logs * Update readme * implements IBM#97 to mask artifactory password * Cli option -prop marked as deprecated Signed-off-by: John Nemec <john.nemec@ibm.com>
* externalize copymodes and allow packaging of additional build logs * Update readme * implements IBM#97 to mask artifactory password * Cli option -prop marked as deprecated Signed-off-by: John Nemec <john.nemec@ibm.com>
This is an enhancement of the current version of the PackageBuildOutputs. The changes propose to
--includeLogs
option or through the propertyincludeLogs
the property file (Comma-separated).deployTypeFilter
in the property file.Please note, that the cli options have changed.
-prop/--propertyFile
for the Artifactory properties should be replaced with-artifactory/--artifactoryPropertiesFile
. Using-prop/--propertyFile
is marked as deprecated and might be removed in the next version.