On Sat, 12 Nov 2022 16:18:54 GMT, Julian Waters <jwat...@openjdk.org> wrote:

>> 8285093 introduced the new UTIL_ARG_WITH definition, which was not available 
>> when both 8282948 and 8282700 were written. They can now be moved to using 
>> the cleaner logic that UTIL_ARG_WITH grants.
>> 
>> There are many more options that still use AC_ARG_WITH in jdk-version.m4. 
>> They are out of the scope of this commit, which aims only to rework the 
>> previous integrated commits mentioned above.
>
> Julian Waters has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into util
>  - Squash

make/autoconf/jdk-version.m4 line 123:

> 121:   # The vendor URL, if any
> 122:   # Only set VENDOR_URL if '--with-vendor-url' was used and is not empty.
> 123:   # Otherwise we will use the value from "branding.conf" included above.

I think this is actually a bit confusing, especially when put in focus like 
this by your rewrite. Perhaps the values in branding.conf should be renamed 
like `DEFAULT_VENDOR_URL`? That is what it is, after all -- even if you modify 
branding.conf in a fork, it can still be overridden by configure arguments.

(This comment is also driven by the fact that I don't really like a 
construction where the same value both goes into the UTIL_ARG_WITH as comes out 
of it. If at all possible, I really like to avoid re-assigning values to 
variables).

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

PR: https://git.openjdk.org/jdk/pull/11020

Reply via email to