C0urante commented on code in PR #12632:
URL: https://github.com/apache/kafka/pull/12632#discussion_r974169402
##########
core/src/main/scala/kafka/tools/StreamsResetter.java:
##########
@@ -213,11 +217,16 @@ private void parseArguments(final String[] args) {
.ofType(String.class)
.describedAs("id")
.required();
- bootstrapServerOption = optionParser.accepts("bootstrap-servers",
"Comma-separated list of broker urls with format: HOST1:PORT1,HOST2:PORT2")
- .withRequiredArg()
+ bootstrapServersOption = optionParser.accepts("bootstrap-servers",
"DEPRECATED: Comma-separated list of broker urls with format:
HOST1:PORT1,HOST2:PORT2")
+ .withOptionalArg()
Review Comment:
I think we still need to revert the change from `withRequiredArg` to
`withOptionalArg`. If something has a required argument, the flag (e.g.,
`--bootstrap-server`) has to be followed by an argument; if it has an optional
argument, you can provide just the flag without any argument.
It's fine if we make providing the flags themselves optional (which I
believe your if/else logic does), but we shouldn't make providing arguments for
those flags optional; if someone provides `--bootstrap-server` or
`--bootstrap-servers`, it should always be followed by the actual bootstrap
server list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
