-
Notifications
You must be signed in to change notification settings - Fork 26
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
DOCSP-34397: improve desc of kafka topic prefix and suffix #147
DOCSP-34397: improve desc of kafka topic prefix and suffix #147
Conversation
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.
Looks pretty good, but I think the wording could be made more clear and more similar to other config descriptions. Left a suggestion that applies to both changes.
@@ -38,8 +45,8 @@ Settings | |||
- | **Type:** string | |||
| | |||
| **Description:** | |||
| The prefix to prepend to database and collection names to | |||
generate the name of the Kafka topic on which to publish the data. | |||
| The string to attach to the beginning of the change event |
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.
Suggestion:
I think this description could be confusing since "attach" is rarely used to describe modifying a programming "string", but is commonly used to describe modifying a cord/rope. I think "concatenate" and "add" are more commonly used, if the intent is to avoid "prepend".
I think it is also helpful to mention that the prefix, database name, and collection name are separated by the character specified in "topic.separator". Maybe something like the following would be better and more similar to "topic.separator":
"
Specifies the beginning of the name of the destination Kafka topic to which the connector publishes change stream events. The destination topic name is composed of the topic.prefix
value, database name, and collection name, separated by the value specified in the topic.separator
setting.
"
Something similar applies to the suffix setting.
* DOCSP-34397: improve desc of kafka topic prefix and suffix * CC suggestion (cherry picked from commit 3555590)
* DOCSP-34397: improve desc of kafka topic prefix and suffix * CC suggestion (cherry picked from commit 3555590)
* DOCSP-34397: improve desc of kafka topic prefix and suffix * CC suggestion (cherry picked from commit 3555590)
* DOCSP-34397: improve desc of kafka topic prefix and suffix * CC suggestion (cherry picked from commit 3555590)
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-34397
Staging - https://preview-mongodbrustagir.gatsbyjs.io/kafka-connector/DOCSP-34397-improve-topic-prefix-desc/source-connector/configuration-properties/kafka-topic/
Self-Review Checklist