On Fri, 12 Nov 2021 11:24:56 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review for this change which implements the enhancement >> noted in https://bugs.openjdk.java.net/browse/JDK-8276559? >> >> The commit in this PR introduces a new `HEAD()` method on the >> `HttpRequest.Builder` interface. Given that this is a public interface on a >> public class of a public module, I decided to add this new method as a >> `default` method to prevent any potential breakages of any non-JDK >> implementations of this interface. The internal >> `jdk.internal.net.http.HttpRequestBuilderImpl` which implements this >> interface, overrides this default implementation to use an implementation of >> its own. >> >> Existing tests have been updated to include coverage for this new method. >> >> P.S: Slightly unrelated question - is it intentional that the `Builder` >> interface specifies the visibility modifiers on the interface methods. For >> example: `public abstract Builder timeout(Duration duration);`, `public >> Builder headers(String... headers);` and so on? > > Jaikiran Pai has updated the pull request incrementally with four additional > commits since the last revision: > > - Implement Daniel's review suggestion - use a different URI for HEAD() > request in HttpRequestNewBuilderTest > - Remove unused imports and fields from HeadTest > - - Update the HeadTest to use the convenience request builder methods GET() > and HEAD() > - Also add @bug tag to updated tests > - fix comment about special casing certain HTTP methods LGTM ------------- Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6348
