On Fri, 18 Oct 2024 12:29:30 GMT, Magnus Ihse Bursie <i...@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

Reply via email to