status: added read from uuid-file #3860
Conversation
rkt/status.go
Outdated
| @@ -30,7 +30,7 @@ import ( | |||
|
|
|||
| var ( | |||
| cmdStatus = &cobra.Command{ | |||
| Use: "status [--wait=bool|timeout] [--wait-ready=bool|timeout] UUID", | |||
| Use: "status [--wait=bool|timeout] [--wait-ready=bool|timeout] --uuid-file=FILE | UUID ...", | |||
There was a problem hiding this comment.
Why the ...? This seems to indicate you can input several UUIDs, but reading the code that's not the case.
rkt/status.go
Outdated
|
|
||
| if len(args) == 0 && flagUUIDFile != "" { |
There was a problem hiding this comment.
This logic allows users to type
# rkt status --uuid-file=/tmp/file 01abcde
And then it'll print the status of 01abcde.
This should not be allowed, if --uuid-file is present we should not accept other arguments, like we do for other commands like rkt stop.
|
Code looks fine to me. Do we maybe want to add functional test covering this? |
rkt/status.go
Outdated
| @@ -30,7 +30,7 @@ import ( | |||
|
|
|||
| var ( | |||
| cmdStatus = &cobra.Command{ | |||
| Use: "status [--wait=bool|timeout] [--wait-ready=bool|timeout] UUID", | |||
| Use: "status [--wait=bool|timeout] [--wait-ready=bool|timeout] --uuid-file=FILE | UUID ", | |||
| @@ -30,7 +30,7 @@ import ( | |||
|
|
|||
| var ( | |||
| cmdStatus = &cobra.Command{ | |||
| Use: "status [--wait=bool|timeout] [--wait-ready=bool|timeout] UUID", | |||
There was a problem hiding this comment.
Thanks for fixing the trailing space! We try to keep commits to what they describe they're doing.
Can you please just remove it from the first commit? Right now it's added on the first commit just to be removed on the second one, that shouldn't be necessary if you don't add it in the first place.
| @@ -28,9 +28,9 @@ function ciSkip { | |||
| exit 0 | |||
| } | |||
|
|
|||
| # Finds the branching point of two commits. | |||
There was a problem hiding this comment.
It's very nice you're fixing these trailing spaces. Can you make this a separate commit though? As I mentioned before, we try to keep commits focused to what they describe they're doing and this change is unrelated to tests.
tests/rkt_status_test.go
Outdated
| @@ -0,0 +1,49 @@ | |||
| // Copyright 2016 The rkt Authors | |||
tests/rkt_status_test.go
Outdated
| ctx := testutils.NewRktRunCtx() | ||
| defer ctx.Cleanup() | ||
|
|
||
| //prepare |
There was a problem hiding this comment.
I think this comment is unnecessary.
|
@iaguis Thanks for reviewing. I have updated the PR. 👍 |
|
I'm fine with this PR now, only one last thing. Can you add the new flag to the documentation in |
|
@iaguis updated. 😄 |
#3005 added --uuid-file to status command