-
Notifications
You must be signed in to change notification settings - Fork 0
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
CLI Unification: Fix miscellaneous typos #1032
Conversation
@@ -186,11 +186,11 @@ func NewKafkaProduceCommand(prerunner cmd.PreRunner) *cobra.Command { | |||
}, | |||
examples.Example{ | |||
Text: "Produce Avro data to a topic called `mytopic5` in Confluent Cloud. Assumes topic has already been created, and Confluent Schema Registry is listening at `http://localhost:8081`.", | |||
Code: `confluent local services kafka produce mytopic5 --cloud --config /tmp/myconfig.properties --value-format avro --property \\\nvalue.schema='{"type":"record","name":"myrecord","fields":[{"name":"f1","type":"string"}]}' \\\n--property schema.registry.url=http://localhost:8081`, |
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.
Oops. I think I wrote this as an intern.
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.
few comments
Example: examples.BuildExampleString( | ||
examples.Example{ | ||
Text: "List all mirrors in the cluster or under the given cluster link.", | ||
Code: "ccloud kafka mirror list --link <link> --mirror-status <mirror-status>", | ||
Text: "List all active mirror topics under `my-link`.", |
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.
mirrored?
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.
That was my first thought too, but "mirror topics" is the right term: https://docs.confluent.io/platform/current/multi-dc-deployments/cluster-linking/mirror-topics-cp.html
@@ -146,16 +145,16 @@ func (c *mirrorCommand) init() { | |||
c.AddCommand(createCmd) | |||
|
|||
promoteCmd := &cobra.Command{ | |||
Use: "promote <destination-topic-1> <destination-topic-2> ... <destination-topic-N> --link <link>", | |||
Short: "Promote the mirror topics.", | |||
Use: "promote <destination-topic-1> <destination-topic-2> ... <destination-topic-N> --link my-link", |
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.
should we use the [] here for optional args?
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.
well i guess the first arg is required, so maybe <> is best
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.
Yeah, I think this way is good for now. Although in the future we should decide on a separate grammar for cases like this.
Checklist
[CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
Did you add/update any commands that accept secrets as args/flags?
What
Fix a few miscellaneous typos found while testing the demo. I need to do a better job of unifying PRs once they're merged into
master
and thenmain
.