showuon commented on code in PR #15252:
URL: https://github.com/apache/kafka/pull/15252#discussion_r1466239773
##########
core/src/main/scala/kafka/admin/ZkSecurityMigrator.scala:
##########
@@ -116,11 +116,10 @@ object ZkSecurityMigrator extends Logging {
try {
run(args)
} catch {
- case e: Exception => {
+ case e: Exception =>
e.printStackTrace()
// must exit with non-zero status so system tests will know we failed
Exit.exit(1)
- }
}
Review Comment:
I'm not completely sure if it'll work as before after removing the curly
brackets. Could you confirm it?
##########
core/src/main/scala/kafka/common/ClientIdAndBroker.scala:
##########
@@ -26,9 +26,5 @@ trait ClientIdBroker {
}
case class ClientIdAndBroker(clientId: String, brokerHost: String, brokerPort:
Int) extends ClientIdBroker {
- override def toString = "%s-%s-%d".format(clientId, brokerHost, brokerPort)
-}
-
-case class ClientIdAllBrokers(clientId: String) extends ClientIdBroker {
- override def toString = "%s-%s".format(clientId, "AllBrokers")
Review Comment:
So `ClientIdAllBrokers` is unused class. Good catch!
##########
core/src/main/scala/kafka/admin/BrokerApiVersionsCommand.scala:
##########
@@ -17,11 +17,14 @@
package kafka.admin
+
+
Review Comment:
What's these for?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
