OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665


##########
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##########
@@ -49,6 +49,7 @@ import 
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
 import org.apache.kafka.common.utils.Sanitizer
 import org.apache.kafka.server.common.AdminOperationException
 import org.apache.kafka.server.config.{ConfigEntityName, ConfigType}
+import org.apache.kafka.server.config.KafkaConfig._

Review Comment:
   <del> For this one and maybe other that import few config from KafkaConfig 
this would be nice improvement but some classes like DynamicBrokerConfig for 
example include a large set of config. I can update few to use  
`KAFKA.SOME_CONFIG` if they need small set of config and leave the others 
specially that we when we move them to java we will do this anyway.</del>
   
   Ignore my previous comment I got the comment wrong. At the moment we can't 
use KafkaConfig everywhere as there's another KafkaConfig in scala



-- 
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

Reply via email to