[GitHub] [lucene-solr] madrob commented on a change in pull request #2275: SOLR-15123: Make all Tool option descriptions follow the same general pattern.

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

madrob commented on a change in pull request #2275:
URL: https://github.com/apache/lucene-solr/pull/2275#discussion_r567953137



##########
File path: solr/core/src/java/org/apache/solr/util/ExportTool.java
##########
@@ -216,32 +216,32 @@ void end() throws IOException {
       Option.builder("url")
           .hasArg()
           .required()
-          .desc("Address of the collection, example http://localhost:8983/solr/gettingstarted")
+          .desc("Address of the collection, example http://localhost:8983/solr/gettingstarted.")

Review comment:
       I think this will be confusing to people if the period is part of the URL or not (if they are copy pasting from a tutorial perhaps.

##########
File path: solr/core/src/java/org/apache/solr/util/PackageTool.java
##########
@@ -261,44 +261,44 @@ protected void runImpl(CommandLine cli) throws Exception {
         .argName("URL")
         .hasArg()
         .required(true)
-        .desc("Address of the Solr Web application, defaults to: " + SolrCLI.DEFAULT_SOLR_URL)
+        .desc("Address of the Solr Web application, defaults to: " + SolrCLI.DEFAULT_SOLR_URL + '.')
         .build(),
 
         Option.builder("collections")
         .argName("COLLECTIONS")
         .hasArg()
         .required(false)
-        .desc("List of collections. Run './solr package help' for more details.")
+        .desc("List of collections.")
         .build(),
 
         Option.builder("cluster")
         .required(false)
-        .desc("Needed to install cluster level plugins in a package. Run './solr package help' for more details.")
+        .desc("Needed to install cluster level plugins in a package.")

Review comment:
       I'm not sure what this description means. There is no verb?

##########
File path: solr/core/src/java/org/apache/solr/util/SolrCLI.java
##########
@@ -1795,25 +1795,25 @@ public ConfigSetUploadTool(PrintStream stdout) {
               .argName("confname") // Comes out in help message
               .hasArg() // Has one sub-argument
               .required(true) // confname argument must be present
-              .desc("Configset name on Zookeeper")
+              .desc("Configset name on Zookeeper.")

Review comment:
       s/on/in maybe?

##########
File path: solr/core/src/java/org/apache/solr/util/SolrCLI.java
##########
@@ -1942,19 +1942,19 @@ public ZkRmTool(PrintStream stdout) {
               .argName("path")
               .hasArg()
               .required(true)
-              .desc("Path to remove")
+              .desc("Path to remove.")
               .build(),
           Option.builder("recurse")
               .argName("recurse")
               .hasArg()
               .required(false)
-              .desc("Recurse (true|false, default is false)")
+              .desc("Recurse (true|false), default is false.")

Review comment:
       These parentheses are inconsistent with the other tools




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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org