RFR: 8340818: Add a new jtreg test root to test the generated documentation [v11]

Erik Joelsson erikj at openjdk.org
Fri Oct 18 12:38:23 UTC 2024


On Fri, 18 Oct 2024 12:29:30 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> It's fine to leave out the conditional, but the double `$$` isn't needed and I'm surprised if it actually works. That must be some lucky double evaluation somewhere in that case. 
>> 
>> Suggestion:
>> 
>>   $1_JTREG_BASIC_OPTIONS += -e:DOCS_JDK_IMAGE_DIR=$(DOCS_JDK_IMAGE_DIR)
>
> I'm not sure why you are saying that. This is in the body of `SetupRunJtregTest`, which will be eval:ed when called, as usual. So `$$` should be used for all variable expansions. It's actually the other way around; `$` will only work due to pure "luck" in this case, and we kind of mis-used that before, splitting the set of variables into two parts, one there `$` would suffice, and one where `$$` were needed. This was just confusing, to save an extra keypress, so I've tried to use `$$` everywhere and always in these macro bodies, for consistency.

Oh, I misread the context. Magnus is correct.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1806421406


More information about the build-dev mailing list