Skip to content
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

'crc delete' message could be improved #760

Closed
cfergeau opened this issue Oct 23, 2019 · 16 comments
Closed

'crc delete' message could be improved #760

cfergeau opened this issue Oct 23, 2019 · 16 comments

Comments

@cfergeau
Copy link
Contributor

After successfully running crc delete, The OpenShift cluster deleted is displayed. This looks like it's missing was.

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019

@robin-owen WDYT?

To me it sounds more passive when 'was' is used. While correct, it is not a necessary change.

@jsliacan
Copy link
Contributor

Then maybe, for consistency, I'd suggest also changing the The OpenShift cluster stopped to The OpenShift cluster was stopped? I see that stopped and deleted are different ("it stopped" makes sense while "it deleted" doesn't). Still, I'd suggest keeping them aligned as much as possible. Notice also that we have The OpenShift cluster is running after start.

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019 via email

@jsliacan
Copy link
Contributor

jsliacan commented Oct 24, 2019

@gbraad agreed! Hence the change which @cfergeau suggests makes sense (in an effort to align the messages). And the message about stopping should also be changed since we're at it - I think. The output currently is:

Stopping the OpenShift cluster, this may take a few minutes...
The OpenShift cluster stopped

The first line suggests that something/someone is stopping the cluster, while the second line suggests that the cluster stopped itself. It'd be better if the second line was The OpenShift cluster was stopped. And this would go well with the The OpenShift cluster was deleted that Christophe suggests, and with The Openshift cluster is running that start prints.

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019 via email

@jsliacan
Copy link
Contributor

Was stopped is an unnecessary addition to indicate that the action happened.

I disagree. The difference is analogous to I was tripped vs I tripped. The former was done by an external agent, in the latter one I was the agent. In our messages, we are mixing these two (passive & active forms).

Anyway, I don't mind either way. Just wanted to say that on top of what @cfergeau suggested, there is also the output of crc stop to look at. Thanks!

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019 via email

@cfergeau
Copy link
Contributor Author

cfergeau commented Oct 24, 2019

I also feel 'was stopped' is better than 'stopped', but I'll just agree with anything @robin-owen suggests :)

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019 via email

@jsliacan
Copy link
Contributor

jsliacan commented Oct 24, 2019

Yeah, sorry I missed it when that was happening! Btw, there's a typo in one of the changes introduced there (existin instead of existing): https://github.com/code-ready/crc/blob/b6c340ad31f47947f017307ac0325aa6dd45060b/pkg/crc/machine/machine.go#L191

err := errors.Newf("VM driver '%s' was requested, but the existin VM is using '%s' instead",

Just leaving it here to go in with any other changes.

@gbraad
Copy link
Contributor

gbraad commented Oct 24, 2019 via email

@cfergeau
Copy link
Contributor Author

Changing strings after release is more difficult, as people might have a script or CI that checks these messages

Fwiw, the real solution for that is to prioritize -o json for all commands, not to have a hard freeze on what we output.

@kowen-rh
Copy link
Contributor

"Deleted the OpenShift cluster" would be an active way of phrasing this.

@gbraad
Copy link
Contributor

gbraad commented Oct 25, 2019

Fwiw, the real solution for that is to prioritize -o json for all commands

this is going to be worked on soon-ish.

Note: but do we really need any confirmation for the delete? I never seen rm or del report back: Removed 100 files and 10 folders. :-/

@gbraad
Copy link
Contributor

gbraad commented Oct 25, 2019

"Deleted the OpenShift cluster" would be an active way of phrasing this.

Right. The passive voice is not wanted here. ... however, is a message needed? In case of an error perhaps, but when confirmation is asked, and nothing fails, you expect this to have happened. Right?

@cfergeau
Copy link
Contributor Author

cfergeau commented Oct 25, 2019

$ crc stop
[WARN] failed to xxx
$

Was the cluster fully stopped, or was the warning fatal? An explicit message avoids this kind of questions.
I agree that it's unusual to explicitly report success, but we had issues filed for confusion similar to the one above.

cfergeau added a commit to cfergeau/crc that referenced this issue Oct 30, 2019
"The OpenShift cluster deleted" was not correct, it was suggested to use
the active voice instead "Deleted the OpenShift cluster"

This fixes crc-org#760
cfergeau added a commit to cfergeau/crc that referenced this issue Oct 30, 2019
"The OpenShift cluster deleted" was not correct, it was suggested to use
the active voice instead "Deleted the OpenShift cluster"

This fixes crc-org#760
cfergeau added a commit to cfergeau/crc that referenced this issue Oct 30, 2019
"The OpenShift cluster deleted" was not correct, it was suggested to use
the active voice instead "Deleted the OpenShift cluster"

This fixes crc-org#760
@gbraad gbraad closed this as completed in 49e925f Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants