[GitHub] [kafka] apovzner commented on a change in pull request #9628: KAFKA-10747: Implement APIs for altering and describing IP connection rate quotas

[GitBox <gi...@apache.org>]

apovzner commented on a change in pull request #9628:
URL: https://github.com/apache/kafka/pull/9628#discussion_r534558407



##########
File path: core/src/main/scala/kafka/admin/ConfigCommand.scala
##########
@@ -56,11 +56,12 @@ import scala.collection._
  *                          --entity-type users --entity-name <user-principal> --entity-type clients --entity-name <client-id>
  *     <li> broker: --broker <broker-id> OR --entity-type brokers --entity-name <broker-id>
  *     <li> broker-logger: --broker-logger <broker-id> OR --entity-type broker-loggers --entity-name <broker-id>
+ *     <li> ip: --IP <ip> OR --entity-type IPs --entity-name <ip>
  * </ul>
- * --entity-type <users|clients|brokers> --entity-default may be specified in place of --entity-type <users|clients|brokers> --entity-name <entityName>
- * when describing or altering default configuration for users, clients, or brokers, respectively.
- * Alternatively, --user-defaults, --client-defaults, or --broker-defaults may be specified in place of
- * --entity-type <users|clients|brokers> --entity-default, respectively.
+ * --entity-type <users|clients|brokers|IPs> --entity-default may be specified in place of --entity-type <users|clients|brokers|IPs> --entity-name <entityName>
+ * when describing or altering default configuration for users, clients, brokers, or IPs, respectively.
+ * Alternatively, --user-defaults, --client-defaults, --broker-defaults, or --IP-defaults may be specified in place of

Review comment:
       yes, agreed about `--IP-defaults` looking bad. 
   
   The main reason I originally proposed upper case "IP" in the KIP is because `ips` as in `--entity-type ips` does not seem very recognizable as plural for IP (but that could be just me), i.e., not very user friendly. But if we do `--entity-type IPs` and then `--ip-defaults`, I think that would cause more issues later because people would assume it's consistent and use wrong case.
   
   Ok, I am convinced that we lower-case is better, since there are more advantages to this vs. upper-case.  




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org