gaurav-narula commented on PR #17460:
URL: https://github.com/apache/kafka/pull/17460#issuecomment-2405828636

   Thanks for your review @mumrah 
   
   > On significant difference is that `Set#of` returns an immutable set. Is it 
acceptable for all of our usages to use an immutable set?
   
   Indeed. Almost all of the usages of `mkSet` are in test files, where we 
usually don't mutate the set. In some cases, where a mutable Set is required, I 
substituted the declaration with `new HashSet<>(List.of(...))`
   
   I could find the following usages of `mkSet` in production code and here are 
some observations:
   
   1. `SslConfigs#RECONFIGURABLE_CONFIGS` and 
`SslConfigs#NON_RECONFIGURABLE_CONFIGS`. There's a getter for the former via 
the `Reconfigurable` interface. I don't think the interface requires the set to 
be mutable as SocketServer.scala implements the same interface and uses a Scala 
Set which is immutable by default.
   2. Topic#INTERNAL_TOPICS where we eventually wrap it in 
`Collections.unmodifiableSet`
   3. JmxReporter#RECONFIGURABLE_CONFIGS - similar to (1)
   4. ScramSaslServer#SUPPORTED_EXTENSIONS - couldn't find any mutation 
operations on the set.
   5. JsonConverter - passed into JsonSerializer/JsonDeserializer. Couldn't 
find any mutation operations on it.
   6. StandardAuthorizerData - couldn't find any mutation operations on the set 
returned
   7. LogConfig - wrapped in `Collections.unmodifiableSet`
   8. ClusterInstance#supportedGroupProtocols() - only used in tests
   


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