On Thu, 3 Aug 2023 12:23:26 GMT, Alan Bateman <[email protected]> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - remove space char
>>  - update the private method javadoc too and minor line length change
>
> src/java.base/share/classes/java/net/InetAddress.java line 791:
> 
>> 789:      * {@linkplain InetAddress#getAddress() IP address} using the 
>> system-wide
>> 790:      * {@linkplain InetAddressResolver resolver}. This is a best effort 
>> method,
>> 791:      * meaning we may not be able to return the fully qualified domain 
>> name; in
> 
> I think you'll need to replace "we" with "it" here. It may also be useful to 
> expand this sentence a bit, maybe with an example of why it may fail. 
> 
> I also wonder (and I'm in two minds on this) is if this method should include 
> an impNote to say what it actually does.

Hello Alan,

> I also wonder (and I'm in two minds on this) is if this method should include 
> an impNote to say what it actually does.

I'm guessing that you are thinking of having the implNote state that a reverse 
name lookup will be performed by this method using the system-wide resolver. 
Right now, the `getHostName()` says this:


* <p>If this InetAddress was created with a host name,
     * this host name will be remembered and returned;
     * otherwise, a reverse name lookup will be performed
     * and the result will be returned based on the system
     * configured resolver. If a lookup of the name service
     * is required, call
     * {@link #getCanonicalHostName() getCanonicalHostName}.


I think we could update the `getCanonicalName()` to mention the reverse name 
lookup. However, I'm unsure if it should be an `implNote` or just formal API 
javadoc.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15134#discussion_r1283138876

Reply via email to