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