[source-kafka] Fix subcommand's long, short, Use values and update docs #65

daisy-ycguo · 2020-07-08T00:48:15Z

Closes #59

navidshaikh · 2020-07-08T05:54:45Z

plugins/source-kafka/pkg/factories/command_factory.go

 	return sourceCmd
 }

 func (f *kafkaSourceCommandFactory) CreateCommand() *cobra.Command {
 	createCmd := f.defaultCommandFactory.CreateCommand()
-	createCmd.Short = "create NAME"
+	createCmd.Use = "create NAME"


this should represent all the flags which are mandatory for creating a kafka source

(check ping source usage string for example where --sink is a mandatory option)

navidshaikh · 2020-07-08T05:55:24Z

plugins/source-kafka/pkg/factories/command_factory.go

-	describeCmd.Short = "describe NAME"
-	describeCmd.Long = "update a Kafka source"
+	describeCmd.Use = "describe NAME"
+	describeCmd.Short = "update a Kafka source"


Suggested change

describeCmd.Short = "update a Kafka source"

describeCmd.Short = "Describe a Kafka source"

navidshaikh · 2020-07-08T05:55:33Z

plugins/source-kafka/pkg/factories/command_factory.go

-	deleteCmd.Short = "delete NAME"
-	deleteCmd.Long = "delete a Kafka source"
+	deleteCmd.Use = "delete NAME"
+	deleteCmd.Short = "delete a Kafka source"


Suggested change

deleteCmd.Short = "delete a Kafka source"

deleteCmd.Short = "Delete a kafka source"

navidshaikh · 2020-07-08T05:55:41Z

plugins/source-kafka/pkg/factories/command_factory.go

 	return sourceCmd
 }

 func (f *kafkaSourceCommandFactory) CreateCommand() *cobra.Command {
 	createCmd := f.defaultCommandFactory.CreateCommand()
-	createCmd.Short = "create NAME"
+	createCmd.Use = "create NAME"
+	createCmd.Short = "create a kafka source"


Suggested change

createCmd.Short = "create a kafka source"

createCmd.Short = "Create a kafka source"

chaozbj · 2020-07-10T04:21:42Z

plugins/source-kafka/pkg/factories/command_factory.go

 	return sourceCmd
 }

 func (f *kafkaSourceCommandFactory) CreateCommand() *cobra.Command {
 	createCmd := f.defaultCommandFactory.CreateCommand()
-	createCmd.Short = "create NAME"
+	createCmd.Use = "create NAME --sink SINK"
+	createCmd.Short = "Create a kafka source"
 	createCmd.Example = `#Creates a new Kafka source named as 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the event messages to service 'event-display'


Should we add a space between # and the example description? I just found kn command has space between them.

Fixed. Thank you.

navidshaikh · 2020-07-10T11:27:43Z

plugins/source-kafka/README.md

+* [kafka create](#kafka-create)	 - Create a kafka source
+* [kafka delete](#kafka-delete)	 - Delete a Kafka source
+* [kafka describe](#kafka-describe)	 - Describe a Kafka source


inconsistent spell for "kafka", should begin with lower case

navidshaikh · 2020-07-10T11:29:36Z

plugins/source-kafka/README.md


 ```
-kafka create NAME [flags]
+kafka create NAME --sink SINK [flags]


I think all the flags except sink are required for kafka source creation.

also, usage should also include being with kn source, i.e.

kn source kafka create NAME --servers SERVERS --topics TOPICS --consumergroup GROUP [options]

I have added all flags to the usage output.
As to add kn source to the usage, it's not easy to fix here. This part of README.md is auto generated by cobra/doc. I could try to change my doc generator tool to add kn source in front of command usage. Let's keep it as it in this PR. I will create a new issue.

navidshaikh · 2020-07-10T11:45:11Z

plugins/source-kafka/README.md

 ```

 #### Examples

 ```
-#Creates a new Kafka source named as 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the event messages to service 'event-display'
+# Creates a new Kafka source named as 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the event messages to service 'event-display'


Suggested change

# Creates a new Kafka source named as 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the event messages to service 'event-display'

# Create a new Kafka source 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the events to service 'event-display'

fixed. Thank you.

navidshaikh · 2020-07-10T11:47:01Z

plugins/source-kafka/pkg/factories/command_factory.go

-	describeCmd.Example = `#Describes a Kafka source with NAME
+	describeCmd.Use = "describe NAME"
+	describeCmd.Short = "Describe a Kafka source"
+	describeCmd.Example = `# Describes a Kafka source with NAME


Suggested change

describeCmd.Example = `# Describes a Kafka source with NAME

describeCmd.Example = `# Describe a Kafka source with NAME

maximilien

I will fix the kn-source-pkg to match some of these Cobra.Long, Cobra.Short, Cobra.Use` usages

knative-prow-robot · 2020-07-13T15:37:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daisy-ycguo

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:

~~plugins/source-kafka/OWNERS~~ [daisy-ycguo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

maximilien

Some changes still needed.

maximilien · 2020-07-16T20:55:47Z

plugins/source-kafka/README.md

-* [kafka delete](#kafka-delete)	 - delete NAME
-* [kafka describe](#kafka-describe)	 - describe NAME
+* [kafka create](#kafka-create)	 - Create a kafka source
+* [kafka delete](#kafka-delete)	 - Delete a Kafka source


Sorry but still seeing "Delete a Kafka source" which is not consistent with the "kafka" spelling. Am I missing something?

change all command output to lower case, except the first word of a sentence.

maximilien · 2020-07-16T20:56:05Z

plugins/source-kafka/README.md

@@ -94,11 +94,11 @@ kn source kafka create mykafkasrc --servers my-cluster-kafka-bootstrap.kafka.svc

 ### kafka delete

-delete NAME
+Delete a Kafka source


Same here...

change all command output to lower case, except the first word of a sentence.

maximilien · 2020-07-22T22:51:16Z

/lgtm

knative-prow-robot requested review from mattmoor and rhuss July 8, 2020 00:48

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 8, 2020

knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 8, 2020

navidshaikh suggested changes Jul 8, 2020

View reviewed changes

daisy-ycguo force-pushed the kafka59 branch from 686ca69 to 6c31460 Compare July 9, 2020 13:52

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 9, 2020

chaozbj reviewed Jul 10, 2020

View reviewed changes

daisy-ycguo force-pushed the kafka59 branch from 6c31460 to a454443 Compare July 10, 2020 09:32

daisy-ycguo changed the title ~~Fix subcommand's long, short, Use values and update docs~~ [source-kafka] Fix subcommand's long, short, Use values and update docs Jul 10, 2020

navidshaikh reviewed Jul 10, 2020

View reviewed changes

maximilien suggested changes Jul 10, 2020

View reviewed changes

maximilien mentioned this pull request Jul 10, 2020

Fix usage of Cobra’s Use, Long, and Short to match kn’s maximilien/kn-source-pkg#20

Closed

daisy-ycguo force-pushed the kafka59 branch from a454443 to 175b859 Compare July 13, 2020 15:37

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2020

maximilien suggested changes Jul 16, 2020

View reviewed changes

Fix subcommand's long, short, Use values and update docs

a9bc6d9

daisy-ycguo force-pushed the kafka59 branch from 175b859 to a9bc6d9 Compare July 20, 2020 13:26

knative-prow-robot assigned maximilien Jul 22, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020

knative-prow-robot merged commit 58a2408 into knative:master Jul 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[source-kafka] Fix subcommand's long, short, Use values and update docs #65

[source-kafka] Fix subcommand's long, short, Use values and update docs #65

daisy-ycguo commented Jul 8, 2020

navidshaikh Jul 8, 2020

daisy-ycguo Jul 9, 2020

navidshaikh Jul 8, 2020

daisy-ycguo Jul 9, 2020

navidshaikh Jul 8, 2020

daisy-ycguo Jul 9, 2020

navidshaikh Jul 8, 2020

daisy-ycguo Jul 9, 2020

chaozbj Jul 10, 2020 •

edited

Loading

daisy-ycguo Jul 10, 2020

navidshaikh Jul 10, 2020

daisy-ycguo Jul 13, 2020

navidshaikh Jul 10, 2020

navidshaikh Jul 10, 2020

daisy-ycguo Jul 13, 2020

navidshaikh Jul 10, 2020

daisy-ycguo Jul 13, 2020

navidshaikh Jul 10, 2020

daisy-ycguo Jul 13, 2020

maximilien left a comment

knative-prow-robot commented Jul 13, 2020

maximilien left a comment

maximilien Jul 16, 2020

daisy-ycguo Jul 20, 2020

maximilien Jul 16, 2020

daisy-ycguo Jul 20, 2020

maximilien commented Jul 22, 2020

	describeCmd.Short = "update a Kafka source"
	describeCmd.Short = "Describe a Kafka source"

	deleteCmd.Short = "delete a Kafka source"
	deleteCmd.Short = "Delete a kafka source"

	createCmd.Short = "create a kafka source"
	createCmd.Short = "Create a kafka source"

	# Creates a new Kafka source named as 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the event messages to service 'event-display'
	# Create a new Kafka source 'mykafkasrc' which subscribes a Kafka server 'my-cluster-kafka-bootstrap.kafka.svc:9092' at topic 'test-topic' using the consumer group ID 'test-consumer-group' and sends the events to service 'event-display'

	describeCmd.Example = `# Describes a Kafka source with NAME
	describeCmd.Example = `# Describe a Kafka source with NAME

[source-kafka] Fix subcommand's long, short, Use values and update docs #65

[source-kafka] Fix subcommand's long, short, Use values and update docs #65

Conversation

daisy-ycguo commented Jul 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chaozbj Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximilien left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Jul 13, 2020

maximilien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximilien commented Jul 22, 2020

chaozbj Jul 10, 2020 •

edited

Loading