On Tue, 20 Feb 2024 18:34:04 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> This is a follow-up to 
>> [JDK-8325877](https://bugs.openjdk.org/browse/JDK-8325877). I was careful in 
>> that bug not to change order of any lines, only split up the original file 
>> into parts.
>> 
>> In this PR, I use the new structure to improve the design. I collect the 
>> functionality into three clearly separate phases, preparation, compilation 
>> and linking. I use consistent naming to reveal whether a function just sets 
>> up variables, or create output. I simplify and split up a few extra hard to 
>> read functions. I move some code around so it is placed more logically.
>> 
>> It can be hard to convince yourself that the changes are correct if you just 
>> look at the end result. I have tried to make small and clear changes in each 
>> separate commit, and to explain in the commit title what I am doing. I 
>> recommend reviewing this PR [commit by 
>> commit](https://github.com/openjdk/jdk/pull/17873/commits).
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Clarify comment based on review

make/common/NativeCompilation.gmk line 132:

> 130: SetupNativeCompilation = $(NamedParamsMacroTemplate)
> 131: define SetupNativeCompilationBody
> 132:   # In this functions, macros named Setup<Foo> are just setting 
> variables.

"functions" is still grammatically wrong. Do you actually mean "this macro", 
"these macros" or "this file"? I think there were a couple of other mentions of 
"function" further down as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17873#discussion_r1496336036

Reply via email to