Allow setting shell and timeout in generatorOptions #497
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Liujingfang1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
| if !f.fSys.IsDir(f.wd) { | ||
| f.wd = filepath.Dir(f.wd) | ||
| if !f.fSys.IsDir(f.wd) { | ||
| return nil, errors.New("not a directory: " + f.wd) | ||
| } | ||
| } | ||
|
|
||
| if options != nil && options.TimeoutSeconds != nil { | ||
| timeout = time.Duration(*options.TimeoutSeconds) * time.Second |
There was a problem hiding this comment.
It seems a non-nil options.TimeoutSeconds always overrides timeout.
It'd be better if these two can be merged into one. Or you need to document it somewhere, maybe in comments.
There was a problem hiding this comment.
The timeout inside secretArgs will be deprecated.
Before the deprecation, what about picking up the max of the two?
There was a problem hiding this comment.
what about picking up the max of the two
That probably makes more sense :)
Liujingfang1
force-pushed
the
generatoroptions
branch
from
8dd1802 to
fdf5fa5
Compare
k8s-ci-robot
added
size/M
k8s-ci-robot
added
the
lgtm
No description provided.