-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add timeout flag for helm #1506
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
@tjerkw thanks for the contribution! This looks good, once you sign the CLA I'll be happy to merge it! |
I signed it |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
I checked manually and @tjerkw has signed the CLA, but @moustafab (who authored the first commit) still has to sign. @moustafab will you be able to sign 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.
Thanks for contributing! Left one comment, which should address #1499 as well.
RecreatePods bool `yaml:"recreatePods,omitempty"` | ||
Wait bool `yaml:"wait,omitempty"` |
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.
It may make more sense to have a Flags []string
field, that way a change doesn't need to be made to skaffold every time a new flag needs to be added.
flags:
- --wait
- --timeout=50
and these can be directly appended to args in helm.go
@priyawadhwa Please see #1507 I would rather just one of these has to be tracked... @tjerkw can you close this one? |
I like the flags approach. It's more maintainable. We should do that
instead of merging these pull requests.
Do the other deploy commands (kubectl for example) also support flags in
the skaffold file?
…On Tue, Jan 29, 2019, 23:21 Moustafa Baiou ***@***.*** wrote:
@priyawadhwa <https://github.com/priyawadhwa> Please see #1507
<#1507> I would
rather just one of these has to be tracked... @tjerkw
<https://github.com/tjerkw> can you close this one?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1506 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAx-sclTm8uERCptCPvhtkPFjhO7FIvAks5vIMl1gaJpZM4aMsZ5>
.
|
Yup, it looks like this |
Closed in favor of #1673 |
No description provided.