You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "osipxd (via GitHub)" <gi...@apache.org> on 2024/10/11 16:25:10 UTC

[PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

osipxd opened a new pull request, #588:
URL: https://github.com/apache/httpcomponents-client/pull/588

   Currently, it is impossible to add  `setHostnameVerificationPolicy` to a call chain


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c merged PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408958891

   LGTM, TY.


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408680195

   @garydgregory Do you want me to revert the commit and ask @osipxd to re-spin the PR with a more conservative fix? 


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408599142

   @garydgregory 
   1. We are using version 0.21.2 of `JApiCmp` which almost the latest.
   2. The change-set does not change the return type. It changes from `void` (no return type)  to a return type. Logically this should compile as before. 


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408654045

   I'm still skeptical, this looks to me like 2 bugs in JApiCmp  😞 
   - The change from non-`final` to `final` breaks subclasses since the containing class is public and non-final.
   - The return type change
   
   After I run:
   ```
   mvn -DskipTests -pl httpclient5 -Dhc.japicmp.version=0.23.0
   ```
   JApiCmp thinks the method is "NEW" and "METHOD_ADDED_TO_PUBLIC_CLASS". Instead, I think it should report "METHOD_RETURN_TYPE_CHANGED". It should also note the change from non-`final` to `final` but it doesn't since it considers the method new, which according to the JLS it is.
   
   There's been a few bugs in this area: https://github.com/siom79/japicmp/issues?q=is%3Aissue++return+
   


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408902649

   @garydgregory @osipxd  Commit reverted and replaced by [891ccc6](https://github.com/apache/httpcomponents-client/commit/891ccc6ce4807bf0fb3120b8006e0600c69e0039)
   
   Please take a look


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408510469

   Cherry-picked to `5.4.x`


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408509126

   @osipxd I suppose I was wrong. The return type is not a part of method name signature and changing return type from void to something else does not cause compile incompatibility. `japicmp` apparently handles the case correctly. 


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2407762314

   @osipxd Damn. This is an API breaking change. We will have to deprecate this method and add another one with a different name. What is even more worrisome is the API breakage has not been detected by the build.


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408590622

   I tried changing the return type on a static method in a different project (Apache Commons IO) and JApiCmp failed with:
   ```
   [ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.23.0:cmp (default-cli) on project commons-io: There is at least one incompatibility: org.apache.commons.io.FileUtils.touch(java.io.File):METHOD_RETURN_TYPE_CHANGED -> [Help 1]
   ```
   We are not using the latest version in HC parent, so this could be a bug in an old version of JApiCmp.
   
   The JLS says that changing a return type is the same as removing and adding a method.
   See [13.4.15. Method Result Type](https://docs.oracle.com/javase/specs/jls/se23/html/jls-13.html#jls-13.4.15)


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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


Re: [PR] Return builder from setHostnameVerificationPolicy [httpcomponents-client]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #588:
URL: https://github.com/apache/httpcomponents-client/pull/588#issuecomment-2408694242

   Hi @ok2c 
   Tl;dr: Yes.
   I think we need to decide whether we want 100% binary compatibility or not. If it were my decision alone I would strive for 100% BC. Jar hell is too painful.


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

To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org

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