-
Notifications
You must be signed in to change notification settings - Fork 949
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
feature: add restart skeleton #890
Conversation
f5952a7
to
408f1a8
Compare
conflicted, please rebase you code ,thanks :) @Letty5411 |
cli/restart.go
Outdated
} | ||
|
||
// runrestart is the entry of restart command. | ||
func (rc *RestartCommand) runrestart(args []string) error { |
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.
s/runrestart/runRestart
cli/restart.go
Outdated
// restartCommand uses to implement 'restart' command, it restarts a container. | ||
type RestartCommand struct { | ||
baseCommand | ||
time int |
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.
s/time/timeout will be more reasonable, WDYT?
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.
ok~
@Letty5411 Maybe we also need add a skeleton of ci test, WDYT? |
cli/restart.go
Outdated
// restartDescription is used to describe restart command in detail and auto generate command doc. | ||
var restartDescription = "restart one or more containers" | ||
|
||
// restartCommand uses to implement 'restart' command, it restarts a container. |
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.
s/restartCommand/RestartCommand
BTW, as your restartDescription says restart one or more containers
, so this comment should change
Signed-off-by: letty <[email protected]>
ping @Letty5411 CI fails according integration system. If this is flaky test, welcome to track this with profiling an issue. build url: https://travis-ci.org/alibaba/pouch/builds/354139016 |
LGTM |
This PR is not the reason CI failed, so I will merge this PR |
Signed-off-by: letty [email protected]
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews