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 2021/01/04 20:07:04 UTC

[GitHub] [httpcomponents-core] arturobernalg opened a new pull request #242: fMinor Improvements:

arturobernalg opened a new pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242


   * remove redundant initialization
   * Add final


----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c merged pull request #242: Minor Improvements:

Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242


   


----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #242: fMinor Improvements:

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242#discussion_r551551690



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/util/Deadline.java
##########
@@ -58,12 +58,12 @@
     /**
      * The maximum (longest-lived) deadline.
      */
-    public static Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
+    public static final Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
 
     /**
      * The minimum (shortest-lived) deadline.
      */
-    public static Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);
+    public static final Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);

Review comment:
       @carterkozak I am pretty sure both Clirr and JApiCmd will mark this change as non-backward compatible. What one might want to do is to deprecate this value in favor of a final variable with a different name.




----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] arturobernalg commented on a change in pull request #242: fMinor Improvements:

Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242#discussion_r551551735



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/util/Deadline.java
##########
@@ -58,12 +58,12 @@
     /**
      * The maximum (longest-lived) deadline.
      */
-    public static Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
+    public static final Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
 
     /**
      * The minimum (shortest-lived) deadline.
      */
-    public static Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);
+    public static final Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);

Review comment:
       reverted. Sorry @ok2c 




----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #242: fMinor Improvements:

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242#discussion_r551565514



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/util/Deadline.java
##########
@@ -58,12 +58,12 @@
     /**
      * The maximum (longest-lived) deadline.
      */
-    public static Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
+    public static final Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
 
     /**
      * The minimum (shortest-lived) deadline.
      */
-    public static Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);
+    public static final Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);

Review comment:
       @ok2c I think the analyzer is telling us that if we make the field final any users who already attempt to replace the default zero value with a different one will be broken, however it's my understanding that would result in other bugs because we expect MIN_VALUE to be zero (and constant) internally. The question is whether or not it's important to continue allowing library consumers to set the field with custom values.




----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c commented on a change in pull request #242: fMinor Improvements:

Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242#discussion_r551548523



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/util/Deadline.java
##########
@@ -58,12 +58,12 @@
     /**
      * The maximum (longest-lived) deadline.
      */
-    public static Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
+    public static final Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
 
     /**
      * The minimum (shortest-lived) deadline.
      */
-    public static Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);
+    public static final Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);

Review comment:
       @arturobernalg Same problem as before. This will likely break binary compatibility with previous 5.x releases.




----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #242: fMinor Improvements:

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #242:
URL: https://github.com/apache/httpcomponents-core/pull/242#discussion_r551550455



##########
File path: httpcore5/src/main/java/org/apache/hc/core5/util/Deadline.java
##########
@@ -58,12 +58,12 @@
     /**
      * The maximum (longest-lived) deadline.
      */
-    public static Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
+    public static final Deadline MAX_VALUE = new Deadline(INTERNAL_MAX_VALUE);
 
     /**
      * The minimum (shortest-lived) deadline.
      */
-    public static Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);
+    public static final Deadline MIN_VALUE = new Deadline(INTERNAL_MIN_VALUE);

Review comment:
       If memory serves, adding a final modifier doesn't break ABI unless something attempts to set the value (but I could be incorrect).
   What if someone _is_ setting an unexpected value here? I think that would be much worse.




----------------------------------------------------------------
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org