You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/23 18:28:24 UTC

[GitHub] [commons-cli] arturobernalg opened a new pull request #56: Minor Improvement:

arturobernalg opened a new pull request #56:
URL: https://github.com/apache/commons-cli/pull/56


   * Unused import
   * Add final
   * License header should be a plain comment
   * Remove Unnecessary boxing


----------------------------------------------------------------
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] [commons-cli] garydgregory commented on a change in pull request #56: Minor Improvements

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#discussion_r549013735



##########
File path: src/main/java/org/apache/commons/cli/CommandLineParser.java
##########
@@ -36,11 +36,11 @@
      */
     CommandLine parse(Options options, String[] arguments) throws ParseException;
 
-    /**

Review comment:
       Leave as Javadoc since one can specify to Javadoc what level of visibility to document for the generated site.
   

##########
File path: src/test/java/org/apache/commons/cli/TypeHandlerTest.java
##########
@@ -77,7 +77,7 @@ public void testCreateValueNumber_Double()
     public void testCreateValueNumber_Long()
         throws Exception
     {
-        assertEquals(Long.valueOf(15), TypeHandler.createValue("15", PatternOptionBuilder.NUMBER_VALUE));
+        assertEquals(15L, TypeHandler.createValue("15", PatternOptionBuilder.NUMBER_VALUE));

Review comment:
       -1 Don't hiding the boxing which is the point of the test, see method name.
   

##########
File path: src/test/java/org/apache/commons/cli/ValuesTest.java
##########
@@ -140,10 +140,10 @@ public void testCharSeparator()
         assertArrayEquals(new String[] { "key", "value" }, cmd.getOptionValues('m'));
     }
 
-    /**
-     * jkeyes - commented out this test as the new architecture
-     * breaks this type of functionality.  I have left the test
-     * here in case I get a brainwave on how to resolve this.
+    /*

Review comment:
       -1 Don't undo Javadoc.
   




----------------------------------------------------------------
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] [commons-cli] arturobernalg commented on pull request #56: Minor Improvement:

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#issuecomment-750766565


   Revert boxing changes


----------------------------------------------------------------
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] [commons-cli] coveralls edited a comment on pull request #56: Minor Improvement:

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#issuecomment-750433412


   
   [![Coverage Status](https://coveralls.io/builds/35931618/badge)](https://coveralls.io/builds/35931618)
   
   Coverage remained the same at 96.358% when pulling **6cab4aa74c7d459a608b0d97203279a8e6045ec9 on arturobernalg:feature/minor_improvement** into **f451a6bdfc25bafd82f5c17eb1831e48cf6e1fba on apache:master**.
   


----------------------------------------------------------------
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] [commons-cli] coveralls commented on pull request #56: Minor Improvement:

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#issuecomment-750433412


   
   [![Coverage Status](https://coveralls.io/builds/35922493/badge)](https://coveralls.io/builds/35922493)
   
   Coverage remained the same at 96.358% when pulling **d44bc2b419d0e5128c53f18f266740f4bbf9a13a on arturobernalg:feature/minor_improvement** into **f451a6bdfc25bafd82f5c17eb1831e48cf6e1fba on apache:master**.
   


----------------------------------------------------------------
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] [commons-cli] arturobernalg commented on pull request #56: Minor Improvements

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#issuecomment-751383968


   Cancel the PR and create a new one


----------------------------------------------------------------
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] [commons-cli] garydgregory commented on pull request #56: Minor Improvement:

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #56:
URL: https://github.com/apache/commons-cli/pull/56#issuecomment-750525780


   -1 please don't change boxing, especially in the tests where we want to make obvious and explicit what we test and how.


----------------------------------------------------------------
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] [commons-cli] arturobernalg closed pull request #56: Minor Improvements

Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #56:
URL: https://github.com/apache/commons-cli/pull/56


   


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