On Thu, 23 Mar 2023 18:30:40 GMT, Mandy Chung <mch...@openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   ARM is 32 bit as per platform.m4 in OpenJDK build
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 823:
> 
>> 821:             if (order != null) {
>> 822:                 this.order = order;
>> 823:                 String targetPlatformVal = 
>> readJavaBaseTargetPlatform(cf);
> 
> Thanks for the update.
> 
> This should also do the fast-path check 
> `isJavaBaseFromCurrentPlatform(javaBasePath)` as in line 838-860.  I suggest 
> to refactor line 838-860 to return the target platform (either runtime 
> platform or from java.base in the given module path).   
> 
> I also suggest to include `endianness` in the `Platform` record class.
> 
> I sent you a patch offline with these suggestions.

Thank you Mandy for the review and the improved patch. I've applied it and 
tests continue to pass. I've updated the PR with these changes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1147050227

Reply via email to