chia7712 commented on code in PR #15823:
URL: https://github.com/apache/kafka/pull/15823#discussion_r1581877008
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java:
##########
@@ -106,7 +106,7 @@ public class ConnectorPluginsResourceTest {
PARTIAL_PROPS.put("name", "test");
PARTIAL_PROPS.put("test.string.config", "testString");
PARTIAL_PROPS.put("test.int.config", "1");
- PARTIAL_PROPS.put("test.list.config", "a,b");
+ PARTIAL_PROPS.put("test.list.config", "a, b");
Review Comment:
ditto. please revert unrelated change
##########
clients/src/main/java/org/apache/kafka/clients/admin/ConsumerGroupDescription.java:
##########
@@ -161,7 +160,7 @@ public Set<AclOperation> authorizedOperations() {
public String toString() {
return "(groupId=" + groupId +
", isSimpleConsumerGroup=" + isSimpleConsumerGroup +
- ", members=" + Utils.join(members, ",") +
+ ", members=" + String.join(",",
Arrays.toString(members.toArray())) +
Review Comment:
ditto
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java:
##########
@@ -188,7 +188,7 @@ public class ConnectorPluginsResourceTest {
partialConfigs.add(configInfo);
configKeyInfo = new ConfigKeyInfo("test.list.config", "LIST", true,
null, "HIGH", "Test configuration for list type.", "Test", 2, "LONG",
"test.list.config", Collections.emptyList());
- configValueInfo = new ConfigValueInfo("test.list.config", "a,b",
asList("a", "b", "c"), Collections.emptyList(), true);
+ configValueInfo = new ConfigValueInfo("test.list.config", "a, b",
asList("a", "b", "c"), Collections.emptyList(), true);
Review Comment:
ditto. please revert unrelated change
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1491,7 +1490,7 @@ public void assign(Collection<TopicPartition> partitions)
{
// See the ApplicationEventProcessor.process() method that handles
this event for more detail.
applicationEventHandler.add(new
AssignmentChangeEvent(subscriptions.allConsumed(), time.milliseconds()));
- log.info("Assigned to partition(s): {}", join(partitions, ", "));
+ log.info("Assigned to partition(s): {}", String.join(", ",
Arrays.toString(partitions.toArray())));
Review Comment:
`Arrays.toString` will add `[]` so that is a bit different to origin
message. Maybe you can use the lambda to generate same message.
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -729,7 +729,7 @@ public void testConfigValidationAllOverride() {
String idempotenceConfigKey =
producerOverrideKey(ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG);
config.put(idempotenceConfigKey, "true");
String bootstrapServersConfigKey =
producerOverrideKey(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG);
- config.put(bootstrapServersConfigKey,
"SASL_PLAINTEXT://localhost:12345,SASL_PLAINTEXT://localhost:23456");
+ config.put(bootstrapServersConfigKey,
"SASL_PLAINTEXT://localhost:12345, SASL_PLAINTEXT://localhost:23456");
Review Comment:
could you please revert this unrelated change?
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -213,11 +211,11 @@ void checkArgs() {
if (options.has(describeOpt)) {
if (!options.has(groupOpt) && !options.has(allGroupsOpt))
CommandLineUtils.printUsageAndExit(parser,
- "Option " + describeOpt + " takes one of these options: "
+ join(allGroupSelectionScopeOpts, ", "));
+ "Option " + describeOpt + " takes one of these options: "
+ String.join(", ", Arrays.toString(allGroupSelectionScopeOpts.toArray())));
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -230,7 +228,7 @@ void checkArgs() {
if (options.has(deleteOpt)) {
if (!options.has(groupOpt) && !options.has(allGroupsOpt))
CommandLineUtils.printUsageAndExit(parser,
- "Option " + deleteOpt + " takes one of these options: " +
join(allGroupSelectionScopeOpts, ", "));
+ "Option " + deleteOpt + " takes one of these options: " +
String.join(", ", Arrays.toString(allGroupSelectionScopeOpts.toArray())));
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java:
##########
@@ -132,7 +132,7 @@ static Set<ConsumerGroupState>
consumerGroupStatesFromString(String input) {
Set<ConsumerGroupState> parsedStates =
Arrays.stream(input.split(",")).map(s ->
ConsumerGroupState.parse(s.trim())).collect(Collectors.toSet());
if (parsedStates.contains(ConsumerGroupState.UNKNOWN)) {
Collection<ConsumerGroupState> validStates =
Arrays.stream(ConsumerGroupState.values()).filter(s -> s !=
ConsumerGroupState.UNKNOWN).collect(Collectors.toList());
- throw new IllegalArgumentException("Invalid state list '" + input
+ "'. Valid states are: " + Utils.join(validStates, ", "));
+ throw new IllegalArgumentException("Invalid state list '" + input
+ "'. Valid states are: " + String.join(", ",
Arrays.toString(validStates.toArray())));
Review Comment:
ditto. please generate the same message as before
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -239,7 +237,7 @@ void checkArgs() {
if (options.has(deleteOffsetsOpt)) {
if (!options.has(groupOpt) || !options.has(topicOpt))
CommandLineUtils.printUsageAndExit(parser,
- "Option " + deleteOffsetsOpt + " takes the following
options: " + join(allDeleteOffsetsOpts, ", "));
+ "Option " + deleteOffsetsOpt + " takes the following
options: " + String.join(", ",
Arrays.toString(allDeleteOffsetsOpts.toArray())));
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -255,7 +253,7 @@ void checkArgs() {
if (!options.has(groupOpt) && !options.has(allGroupsOpt))
CommandLineUtils.printUsageAndExit(parser,
- "Option " + resetOffsetsOpt + " takes one of these
options: " + join(allGroupSelectionScopeOpts, ", "));
+ "Option " + resetOffsetsOpt + " takes one of these
options: " + String.join(", ",
Arrays.toString(allGroupSelectionScopeOpts.toArray())));
Review Comment:
ditto
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -213,11 +211,11 @@ void checkArgs() {
if (options.has(describeOpt)) {
if (!options.has(groupOpt) && !options.has(allGroupsOpt))
CommandLineUtils.printUsageAndExit(parser,
- "Option " + describeOpt + " takes one of these options: "
+ join(allGroupSelectionScopeOpts, ", "));
+ "Option " + describeOpt + " takes one of these options: "
+ String.join(", ", Arrays.toString(allGroupSelectionScopeOpts.toArray())));
List<OptionSpec<?>> mutuallyExclusiveOpts =
Arrays.asList(membersOpt, offsetsOpt, stateOpt);
if (mutuallyExclusiveOpts.stream().mapToInt(o -> options.has(o) ?
1 : 0).sum() > 1) {
CommandLineUtils.printUsageAndExit(parser,
- "Option " + describeOpt + " takes at most one of these
options: " + join(mutuallyExclusiveOpts, ", "));
+ "Option " + describeOpt + " takes at most one of these
options: " + String.join(", ",
Arrays.toString(mutuallyExclusiveOpts.toArray())));
Review Comment:
ditto
--
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
