-
Notifications
You must be signed in to change notification settings - Fork 261
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
Addresses issue #102 #103
Addresses issue #102 #103
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: maximilien If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -53,7 +53,7 @@ func NewServiceDeleteCommand(p *KnParams) *cobra.Command { | |||
if err != nil { | |||
return err | |||
} | |||
fmt.Fprintf(cmd.OutOrStdout(), "Deleted %s in %s namespace.\n", args[0], namespace) | |||
fmt.Fprintf(cmd.OutOrStdout(), "Deleted '%s' in '%s' namespace.\n", args[0], namespace) |
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.
Could we please add 'service' as extra context info (as this is easier to detect e.g. when grepping in logs when this output is redirected) ? Also, would prefer to have "namespace" before the actual namespace (as this is the usual order: in message.
I.e. Deleted service '%s' in namespace '%s'
Whether to add punctuation or not should be follow a common convention over all messages (not sure how it is in the client).
This is addressed in PR #95. |
Closing since already addressed in #95 |
Changes
service delete
output to be nicer to something like:Service 'hello' in namespace 'default' deleted