showuon commented on PR #15273: URL: https://github.com/apache/kafka/pull/15273#issuecomment-1920457353
> @showuon I rebased and addressed your comments. Did not remove properties definitions from KafkaConfig, because this breaks a lot of tests. I think this should be done as part of KafkaConfig migration. In the end it is a small amount of duplication, but properties are always defined in one place. Thanks @fvaleri ! I had another look, and still think the duplication is not a great idea. It might cause potential issue someday, even if it looks good now. From what I can see, almost all the config validation exists in KafkaConfig, so could we remove the config definition in LogConfig? My imagination is the LogConfig can do something similar as RaftConfig, that has no config definition there. If you want to have a copy of the config value, you could pass in KafkaConfig instance into LogConfig [here](https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/log/LogManager.scala#L1513). Do you think that make sense? -- 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
