On Tue, 15 Mar 2022 23:50:20 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [x] java.base
>> - [x] java.desktop
>> - [x] jdk.compiler
>> - [x] java.se
>
> Magnus Ihse Bursie has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into shuffle-data-reborn
>  - Fix merge
>  - Merge tag 'jdk-19+13' into shuffle-data-reborn
>    
>    Added tag jdk-19+13 for changeset 5df2a057
>  - Move characterdata templates to share/classes/java/lang.
>  - Update comment refering to "make" dir
>  - Move new symbols to jdk.compiler
>  - Merge branch 'master' into shuffle-data
>  - Move macosxicons from share to macosx
>  - Move to share/data, and move jdwp.spec to java.se
>  - Update references in test
>  - ... and 2 more: 
> https://git.openjdk.java.net/jdk/compare/83d77186...598f740f

Looks good, with minor comments. Also I am assuming you will modify the 
copyright year for these files before the merge.

make/modules/jdk.charsets/Gensrc.gmk line 32:

> 30: # Generate files using the charsetmapping tool
> 31: #
> 32: CHARSET_DATA_DIR := $(TOPDIR)/src/java.base/share/data/charsetmapping

Is it intentional to leave `java.base` literal here, or should it be replaced 
with `$(MODULE_SRC)`? I see this inconsistency in other tools' `gensrc.gmk` too

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

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1611

Reply via email to