AndrewJSchofield commented on code in PR #20431: URL: https://github.com/apache/kafka/pull/20431#discussion_r2308006250
########## tools/src/main/java/org/apache/kafka/tools/ClusterTool.java: ########## @@ -82,9 +83,13 @@ static void execute(String... args) throws Exception { connectionOptions.addArgument("--bootstrap-controller", "-C") .action(store()) .help("A list of host/port pairs to use for establishing the connection to the KRaft controllers."); - subpparser.addArgument("--config", "-c") + subpparser.addArgument("--config") .action(store()) - .help("A property file containing configurations for the Admin client."); + .help("(DEPRECATED) A property file containing configurations for the Admin client." + Review Comment: nit: Missing space between sentences. ########## tools/src/main/java/org/apache/kafka/tools/LeaderElectionCommand.java: ########## @@ -260,10 +267,18 @@ public LeaderElectionCommandOptions(String[] args) { adminClientConfig = parser .accepts( "admin.config", - "Configuration properties files to pass to the admin client") + "(DEPRECATED) Configuration properties files to pass to the admin client" + + "This option will be removed in a future version. Use --command-config instead.") .withRequiredArg() .describedAs("config file") .ofType(String.class); + adminClientCommandConfig = parser Review Comment: Maybe `commandConfig` would be a bit less cumbersome. ########## tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java: ########## @@ -624,7 +631,12 @@ public StreamsResetterOptions(String[] args) { .withRequiredArg() .describedAs("number-of-offsets") .ofType(Long.class); - commandConfigOption = parser.accepts("config-file", "Property file containing configs to be passed to admin clients and embedded consumer.") + configOption = parser.accepts("config-file", "(DEPRECATED) Property file containing configs to be passed to admin clients and embedded consumer." Review Comment: nit: Missing space between sentences in help message. ########## tools/src/main/java/org/apache/kafka/tools/LeaderElectionCommand.java: ########## @@ -260,10 +267,18 @@ public LeaderElectionCommandOptions(String[] args) { adminClientConfig = parser .accepts( "admin.config", - "Configuration properties files to pass to the admin client") + "(DEPRECATED) Configuration properties files to pass to the admin client" + + "This option will be removed in a future version. Use --command-config instead.") .withRequiredArg() .describedAs("config file") .ofType(String.class); + adminClientCommandConfig = parser + .accepts( + "command-config", + "Config properties file to pass to the admin client") Review Comment: nit: Missing `.` at the end of the help message. ########## tools/src/main/java/org/apache/kafka/tools/LeaderElectionCommand.java: ########## @@ -260,10 +267,18 @@ public LeaderElectionCommandOptions(String[] args) { adminClientConfig = parser .accepts( "admin.config", - "Configuration properties files to pass to the admin client") + "(DEPRECATED) Configuration properties files to pass to the admin client" + + "This option will be removed in a future version. Use --command-config instead.") .withRequiredArg() .describedAs("config file") .ofType(String.class); + adminClientCommandConfig = parser + .accepts( + "command-config", + "Config properties file to pass to the admin client") + .withRequiredArg() + .describedAs("Config properties files") Review Comment: I would describe it as `"config file"`. It certainly isn't multiple files. -- 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