-
Notifications
You must be signed in to change notification settings - Fork 717
Use --start flag to start VM for 'clone' and 'edit' commands #4108
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
Conversation
2e4240b to
9e19785
Compare
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.
Please wait for feedback from @AkihiroSuda before making any changes.
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.
I still think we should remove --yes from being a global option that shows up in limactl help. start, edit, and clone are the only commands that check the value of the --tty setting, so it really only applies to start now.
So I would make it a local option in those 3 commands, and make it hidden in edit and clone, so it really becomes just a start option.
@AkihiroSuda You seemed to have reservations about making the deprecated option hidden. What is the reason for that?
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.
You seemed to have reservations about making the deprecated option hidden. What is the reason for that?
Because we have been keeping other deprecated commands/options.
(limactl show-ssh)
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.
Yes, but I don't understand why this makes sense: the point of deprecation is to transition away from something because it will go away. Keeping it available for new users to discover and use is not helpful.
Only users that already know about these commands (or existing scripts) should continue to use them, but get a warning, so they can transition away before the commands get removed. If there is no intention of removing a command or feature, then there is no point in deprecating 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.
So I would make it a local option in those 3 commands, and make it hidden in
editandclone, so it really becomes just astartoption.
We definitely need to make it a local option if we don't want to hide the deprecated option. That way we can mark it in the --help output for edit and clone as deprecated (which it isn't right now):
❯ l edit --help Edit an instance of Lima or a template ... Global Flags: ... -y, --yes Alias of --tty=false
But also so that --yes doesn't show up for every single subcommand when it really is just an option for start anymore.
One big reason for getting rid of --yes is to avoid user confusion, and we are not achieving this by keeping it very visible.
cmd/limactl/main.go
Outdated
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.
I don't really understand the warning. --start has different semantics than --yes, so --start=false may be the same as --yes=true, but --start=true (start without asking) is different from --yes=false (ask about starting).
I think the edit and clone commands should just print the simplified message:
Users can then run limactl edit --help to learn more.
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.
What about:
9e19785 to
c57a0c8
Compare
cmd/limactl/main.go
Outdated
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.
Now "rename" has to be covered too
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.
Please resolve merge conflicts and add support for the limactl rename command as well.
@Horiodino Could you update the PR?
c57a0c8 to
fb047aa
Compare
Signed-off-by: Praful Khanduri <holiodin@gmail.com>
fb047aa to
1e368f3
Compare
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
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!
I'm going to approve and merge this to get the --start option into the Lima 2.0 release.
But I don't think it has solved the confusion around --yes being a global option for all commands. But we can address this later; it is mostly about deprecating/hiding it from the --help output.
This PR addresses [Issue #3995] regarding the confusing and inconsistent behavior of the --yes flag in limactl clone and limactl edit. The following changes have been made:
Changes
--yesflag for the clone and edit commands. A warning is shown when used, recommending--startinstead.--startflag (default: true) to both commands, allowing users to explicitly control whether the instance is started after cloning or editing.