You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/28 06:00:51 UTC

[GitHub] [pulsar] 315157973 opened a new pull request #9750: Keep the command style consistent

315157973 opened a new pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750


   
   ### Motivation
   There is no `-per-topic` suffix for topic-level, for example:
   namespace-level: get-max-producers-per-topic
   topic-level: get-max-producers
   
   But `get-max-subscriptions-per-topic` now has a suffix, which should be consistent with the rules of other commands.
   
   Since this interface has just been added and has not been released yet, there is no compatibility issue
   
   
   


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



[GitHub] [pulsar] 315157973 commented on a change in pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750#discussion_r584418067



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
##########
@@ -382,7 +382,7 @@ void run() throws PulsarAdminException {
         @Parameter(description = "tenant/namespace", required = true)
         private java.util.List<String> params;
 
-        @Parameter(names = { "--maxSubscriptionsPerTopic", "-m" }, description = "Max subscriptions per topic",
+        @Parameter(names = { "--max-subscriptions-per-topic", "-m" }, description = "Max subscriptions per topic",

Review comment:
       @codelipenghui 




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



[GitHub] [pulsar] 315157973 commented on a change in pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750#discussion_r584386554



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
##########
@@ -382,7 +382,7 @@ void run() throws PulsarAdminException {
         @Parameter(description = "tenant/namespace", required = true)
         private java.util.List<String> params;
 
-        @Parameter(names = { "--maxSubscriptionsPerTopic", "-m" }, description = "Max subscriptions per topic",
+        @Parameter(names = { "--max-subscriptions-per-topic", "-m" }, description = "Max subscriptions per topic",

Review comment:
       topic-level `get-max-subscriptions-per-topic` was added on 2020-12-16
   namespace-level `maxSubscriptionsPerTopic` was added on 2020-12-12
   
   It seems that it has not been released yet, can it be modified directly?




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



[GitHub] [pulsar] codelipenghui commented on a change in pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750#discussion_r584383280



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
##########
@@ -382,7 +382,7 @@ void run() throws PulsarAdminException {
         @Parameter(description = "tenant/namespace", required = true)
         private java.util.List<String> params;
 
-        @Parameter(names = { "--maxSubscriptionsPerTopic", "-m" }, description = "Max subscriptions per topic",
+        @Parameter(names = { "--max-subscriptions-per-topic", "-m" }, description = "Max subscriptions per topic",

Review comment:
       Could you please deprecate the old parameter name instead of deleting it directly? We can make the old one does not appear in the CLI help but also able to use it. Then we can try to delete it in the 2.8.0 or later.




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



[GitHub] [pulsar] codelipenghui merged pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750


   


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



[GitHub] [pulsar] codelipenghui commented on a change in pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750#discussion_r584447075



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
##########
@@ -382,7 +382,7 @@ void run() throws PulsarAdminException {
         @Parameter(description = "tenant/namespace", required = true)
         private java.util.List<String> params;
 
-        @Parameter(names = { "--maxSubscriptionsPerTopic", "-m" }, description = "Max subscriptions per topic",
+        @Parameter(names = { "--max-subscriptions-per-topic", "-m" }, description = "Max subscriptions per topic",

Review comment:
       Ok




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



[GitHub] [pulsar] 315157973 commented on pull request #9750: Keep the command style consistent

Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #9750:
URL: https://github.com/apache/pulsar/pull/9750#issuecomment-787442336


   /pulsarbot run-failure-checks


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