You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2019/12/25 16:51:38 UTC

[GitHub] [httpcomponents-core] michael-o opened a new pull request #171: Consistency fixes

michael-o opened a new pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171
 
 
   As discussed on the list. Here are the points mentioned broken down to separate commits.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569108447
 
 
   Changes merged except `LangUtils` removal.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569070023
 
 
   > 
   > 
   > > @michael-o I am fine with everything except for [21fb5c1](https://github.com/apache/httpcomponents-core/commit/21fb5c1fb341e224abc7cec1d913e03f4c7d012e). I would prefer to keep `LangUtils` until 5.1.
   > 
   > If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.
   
   I actually had the same question. I don't mind keeping it, but I see no benefit in doing so.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569078777
 
 
   > 
   > 
   > > If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.
   > 
   > @michael-o We do not need to drop it. We can deprecate it and stop using it. What I dislike about `Objects` is that hideous `static int hash(Object... values)` with an extra array allocation due to vararg signature. I would like to upgrade to JRE 8 or 11 and then consider deprecating `LangUtils` in favor of `Objects`
   
   Now I understand, makes sense. How would Java 8 or 11 make it any better? Those has methods exist since Java 7.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569072956
 
 
   I have moved the LangUtils removal on top to be easily moved to a separate branch if it shall be retained for any reason in 5.0.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
ok2c commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569079185
 
 
   > > > If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.
   > > 
   > > 
   > > @michael-o We do not need to drop it. We can deprecate it and stop using it. What I dislike about `Objects` is that hideous `static int hash(Object... values)` with an extra array allocation due to vararg signature. I would like to upgrade to JRE 8 or 11 and then consider deprecating `LangUtils` in favor of `Objects`
   > 
   > Now I understand, makes sense. How would Java 8 or 11 make it any better? Those has methods exist since Java 7.
   
   @michael-o I recall reading that _possibly_ some newer JVMs include a special optimization routine specifically for `Objects#hash` but it had no time to investigate it.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] garydgregory commented on a change in pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#discussion_r361338274
 
 

 ##########
 File path: httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/LengthDelimitedDecoder.java
 ##########
 @@ -86,7 +86,7 @@ public int read(final ByteBuffer dst) throws IOException {
             setCompleted();
             if (this.len < this.contentLength) {
                 throw new ConnectionClosedException(
-                                "Premature end of Content-Length delimited message body (expected: %,d; received: %,d)",
+                                "Premature end of Content-Length delimited message body (expected: %d; received: %d)",
 
 Review comment:
   I do not understand why you'd want to make the message less readable by humans.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o closed pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o closed pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171
 
 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569079277
 
 
   > 
   > 
   > > > > If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.
   > > > 
   > > > 
   > > > @michael-o We do not need to drop it. We can deprecate it and stop using it. What I dislike about `Objects` is that hideous `static int hash(Object... values)` with an extra array allocation due to vararg signature. I would like to upgrade to JRE 8 or 11 and then consider deprecating `LangUtils` in favor of `Objects`
   > > 
   > > 
   > > Now I understand, makes sense. How would Java 8 or 11 make it any better? Those has methods exist since Java 7.
   > 
   > @michael-o I recall reading that _possibly_ some newer JVMs include a special optimization routine specifically for `Objects#hash` but it had no time to investigate it.
   
   Alright, will move it out and merge the rest to 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
ok2c commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569074724
 
 
   > If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.
   
   @michael-o We do not need to drop it. We can deprecate it and stop using it. What I dislike about `Objects` is that hideous `static int hash(Object... values)` with an extra array allocation due to vararg signature. I would like to upgrade to JRE 8 or 11 and then consider deprecating `LangUtils` in favor of `Objects` 

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
ok2c commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569018621
 
 
   @michael-o I am fine with everything except for 21fb5c1. I would prefer to keep `LangUtils` until 5.1.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#discussion_r361341277
 
 

 ##########
 File path: httpcore5/src/main/java/org/apache/hc/core5/util/Args.java
 ##########
 @@ -114,7 +114,7 @@ private static IllegalArgumentException illegalArgumentExceptionNotNull(final St
 
     public static <T extends CharSequence> T notEmpty(final T argument, final String name) {
         if (argument == null) {
-            throw illegalArgumentExceptionNotNull(name);
+            throw NullPointerException(name);
 
 Review comment:
   Sure, why not. I kept this for consistency with the rest of the class.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] garydgregory commented on a change in pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#discussion_r361338303
 
 

 ##########
 File path: httpcore5/src/main/java/org/apache/hc/core5/util/Args.java
 ##########
 @@ -114,7 +114,7 @@ private static IllegalArgumentException illegalArgumentExceptionNotNull(final St
 
     public static <T extends CharSequence> T notEmpty(final T argument, final String name) {
         if (argument == null) {
-            throw illegalArgumentExceptionNotNull(name);
+            throw NullPointerException(name);
 
 Review comment:
   Why not use `Objects.requireNonNull()`?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] garydgregory commented on issue #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
garydgregory commented on issue #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#issuecomment-569068203
 
 
   > @michael-o I am fine with everything except for [21fb5c1](https://github.com/apache/httpcomponents-core/commit/21fb5c1fb341e224abc7cec1d913e03f4c7d012e). I would prefer to keep `LangUtils` until 5.1.
   
   If a public API is released in 5.x, we cannot drop it until 6.0, we should drop it now.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#discussion_r361341277
 
 

 ##########
 File path: httpcore5/src/main/java/org/apache/hc/core5/util/Args.java
 ##########
 @@ -114,7 +114,7 @@ private static IllegalArgumentException illegalArgumentExceptionNotNull(final St
 
     public static <T extends CharSequence> T notEmpty(final T argument, final String name) {
         if (argument == null) {
-            throw illegalArgumentExceptionNotNull(name);
+            throw NullPointerException(name);
 
 Review comment:
   Sure, why not.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #171: Consistency fixes

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #171: Consistency fixes
URL: https://github.com/apache/httpcomponents-core/pull/171#discussion_r361341264
 
 

 ##########
 File path: httpcore5/src/main/java/org/apache/hc/core5/http/impl/nio/LengthDelimitedDecoder.java
 ##########
 @@ -86,7 +86,7 @@ public int read(final ByteBuffer dst) throws IOException {
             setCompleted();
             if (this.len < this.contentLength) {
                 throw new ConnectionClosedException(
-                                "Premature end of Content-Length delimited message body (expected: %,d; received: %,d)",
+                                "Premature end of Content-Length delimited message body (expected: %d; received: %d)",
 
 Review comment:
   As discussed, it is not consistent across locales. Thus the messages are never stable.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org