Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

rkt/stop: implement --uuid-file #2902

Merged
merged 4 commits into from
Jul 12, 2016
Merged

rkt/stop: implement --uuid-file #2902

merged 4 commits into from
Jul 12, 2016

Conversation

iaguis
Copy link
Member

@iaguis iaguis commented Jul 11, 2016

So the user can use the value saved on rkt run with --uuid-file-save.

Fixes #2892

@alban
Copy link
Member

alban commented Jul 11, 2016

test?

podUUID, err := resolveUUID(uuid)
ret := 0
switch {
case len(args) == 0 && flagUUIDFile != "":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it is either UUID or --uuid-file=FILE? Why not both? In this case, the usage in cobra should be updated.

Use:   "stop --uuid-file=FILE | UUID ...",

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I noticed that, we should change it in rkt rm too

We allow either `UUID` or `--uuid-file`, not both. Reflect that on the
help string.
So the user can use the value saved on rkt run with --uuid-file-save.
It's already included by the log package.
@iaguis
Copy link
Member Author

iaguis commented Jul 11, 2016

Updated.

@lucab
Copy link
Member

lucab commented Jul 11, 2016

As you are touching it, I think it makes sense to also align rkt run into using --uuid-file and deprecating current --uuid-file-save.

@iaguis
Copy link
Member Author

iaguis commented Jul 11, 2016

Since one is reading the file and the other is saving it I'm not sure they should have the same name...

cc @robszumski

@lucab
Copy link
Member

lucab commented Jul 12, 2016

@iaguis I see my suggestion was not un-controversial as I was expecting, so let's wait for a tie-breaking input from @robszumski on it.

In the meanwhile, we can decouple this discussion from landing this. Current PR looks fine as-is, flag renaming (if accepted) can be done separately.

@lucab lucab self-assigned this Jul 12, 2016
@lucab lucab merged commit bfc6889 into rkt:master Jul 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants