-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[jib-cli-jar] Add option to specify creation time of container #3050
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.
Can we add a test that checks the default time of the built image (or something to that effect) when the option was not given?
Codecov Report
@@ Coverage Diff @@
## master #3050 +/- ##
============================================
+ Coverage 71.07% 71.09% +0.01%
- Complexity 2305 2307 +2
============================================
Files 278 278
Lines 9751 9755 +4
Branches 989 989
============================================
+ Hits 6931 6935 +4
Misses 2480 2480
Partials 340 340
Continue to review full report at Codecov.
|
@CommandLine.Option( | ||
names = "--creation-time", | ||
paramLabel = "<creation-time>", | ||
description = |
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.
Another option is to provide the default (e.g., 0?). That way, maybe creationTime
can never be null?
Either way has pros and cons though.
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 did think about adding a default but I assumed this would be a little redundant since we already have a default value for this field in jib?
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.
Either is fine. At least we can document the default value in the option description.
Required a bit of refactoring to use the
Instants
class which was initially implemented for the build command.