Request for assistance: Verify and update mailing list filter rules for jdk/jdk in preparation for Skara transition
david.holmes at oracle.com
Thu Aug 27 22:00:30 UTC 2020
To expand further on my comments. An initial coarse mapping of file to
mailing-list is a good thing and yes it addresses the new-contributor
problem (or at least assists with it). And as Jesper states we have that
mapping already written down here at Oracle.
But what has been produced is unnecessarily complicated IMO and results
in many unnecessary cross-postings. Rather than going through a long
complex mapping and culling the inappropriate ones, it would have been
easier IMO to add in missing ones to a coarser starting list. I see many
issues with the proposed mapping generated e.g.
- why do build-dev care about manpage edits?
- why does compiler-dev (javac) care about hotspot changes or core-libs
changes? I monitor core-libs-dev and also review there and it is very
rare to see something reviewed on core-libs-dev that is also reviewed on
compiler-dev, yet the rules include
src/java.base/share/classes/java/lang/ for compiler-dev!
I'm all for aiding the developer selecting the right set of MLs, but
that has to be a guided choice where the developer has the control IMO
(and Jesper's :) ). At the moment the developer will be able to add to
the list of MLs in a PR but not remove the ones produced by this mapping
- though Erik D. has stated that he will look into that. Though I'm
concerned that the only practical way to produce a PR will be via the
GUI rather than the CLI.
On 28/08/2020 5:32 am, Jesper Wilhelmsson wrote:
> Hi Robin et al.
> There is a mapping available already on our internal wiki that you can use/could have used as a starting point, search for "How to know who owns what code".
> I intend to move this mapping to the Developers' Guide fairly soon so that it's available to the whole community.
> Automating the selection of mailing lists is good iff the author of a change is programmatically required to verify that the selected lists is correct. For the user who don't have a clue as to what list to send to verifying won't make any difference, but not being forced to verify will allow the user who do know to become lazy and trust the automatic selection.
> An automated system can never be 100% correct since changes to the same file can be done for different reasons and require review on different lists, so regardless of where the initial set of addresses come from the author must be able to edit these to add or remove lists to fit the change in question.
>> On 27 Aug 2020, at 19:37, Joe Darcy <joe.darcy at oracle.com> wrote:
>> The mapping of changed-file-to-OpenJDK-mailing-list(s)-to-review-the-file can be non-obvious, both to new contributors and to experienced contributors working in an unfamiliar area. If an automated mapping gets this 95+% correct, with the ability for manual tweaking, that strikes me as an overall improvement over the current situation.
>> The current mapping has various entries that should be changed, but that is why it is being sent out for review before the Skara transition :-)
>> As a general comment, I would expect both the mapping and other aspects of the Skara tooling to get updated in response to feedback after jdk/jdk moves over.
>> On 8/27/2020 9:53 AM, Igor Ignatyev wrote:
>>> Hi Robin,
>>> although I really like the idea of having mapping b/w files and groups/components/subcomponents, I agree w/ David that in its *current* form it's unworkable. having the mapping in Skara repo is impractical, as it introduces additional overhead for maintenance, not to speak of possible desynchronization. Thus I suggest moving the mapping to jdk/jdk repo and updating Skara tooling accordingly.
>>> I also have a question regarding the format, why did you decide to invent your own format instead of using CODEOWNER-like format which fits the needs rather nicely and is used for similar purposes by github and gitlab?
>>> -- Igor
>>>> On Aug 27, 2020, at 6:26 AM, David Holmes <david.holmes at oracle.com> wrote:
>>>> In all seriousness I just don't think this is a reasonable or necessary thing to do. When you create a PR you should specify the mailing lists to be used, as you do today with a RFR. Trying to maintain a file by file mapping is just a huge initial setup cost and a maintenance nightmare. It is not necessary to try and automate this IMO.
>>>> I wish this intent had been flagged/discussed some time ago. :(
>>>> On 27/08/2020 8:34 pm, Robin Westberg wrote:
>>>>> Hi all,
>>>>> As part of transitioning the jdk/jdk repository to Git with project Skara, we have created a set of rules that determine which mailing list(s) should be the default recipient of a review request, depending on which files are changed. The initial version of these rules was created by looking at historical commits and matching them with existing mailing list review threads. This has produced a reasonable basis, but it can most certainly be made better with some additional manual curating.
>>>>> Therefore, it would be very helpful if people with good knowledge of the various subsystems and source files that make up the JDK would be willing to take a look at these rules, and also suggest improvements where needed. In addition, lists like  would also be very useful insofar they exist.
>>>>> The current version of these rules is located in a JSON file in the Skara repository at . In order to check the validity of the rules, there is also a CLI tool that can be used to apply it to either a subset of files or existing commits and produce a suggestion  . If you are interested in helping out with curating these rules, these are the steps to get started:
>>>>> 1. Install the Skara CLI tools: https://wiki.openjdk.java.net/display/SKARA/CLI+Tools
>>>>> 2. Locate a suitable clone of the jdk/jdk repository (either Mercurial or Git is fine)
>>>>> 3. Change (cd) to the root of your jdk/jdk repository
>>>>> 3. Run the “debug mlrules” command on your favorite subset of files, for example like this (use the actual location of jdk.json on your system):
>>>>> $ git skara debug mlrules -v ~/git/skara/config/mailinglist/rules/jdk.json src/hotspot/share/jfr/
>>>>> Reading rules file...
>>>>> src/hotspot/share/jfr/dcmd: [hotspot-jfr-dev]
>>>>> src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp: [hotspot-jfr-dev]
>>>>> src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.hpp: [hotspot-jfr-dev]
>>>>> src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp: [hotspot-jfr-dev, serviceability-dev]
>>>>> Final list suggestion is: [hotspot-jfr-dev, serviceability-dev]
>>>>> The command accepts multiple folder and/or file names to make it possible to simulate a potential change to a given set of files:
>>>>> $ git skara debug mlrules -v ../skara/config/mailinglist/rules/jdk.json doc/ide.md src/hotspot/cpu/x86/x86.ad src/hotspot/os/linux/gc/z/zNUMA_linux.cpp
>>>>> Reading rules file...
>>>>> doc: [build-dev]
>>>>> src/hotspot/cpu: [hotspot-compiler-dev]
>>>>> src/hotspot/os: [hotspot-runtime-dev, hotspot-gc-dev]
>>>>> Combined list suggestion: [build-dev, hotspot-compiler-dev, hotspot-gc-dev, hotspot-runtime-dev]
>>>>> Final list suggestion is: [build-dev, hotspot-dev]
>>>>> If the suggestions look fine, all is well. If not, you are welcome to propose a change to the rules, preferably by editing the jdk.json file  and creating a pull request towards the Skara project as described in . Coincidentally, this is the same way that future changes to the jdk/jdk repository will be integrated, so this exercise could also serve as a way of getting started with Git / Skara!
>>>>> Best regards,
>>>>>  https://openjdk.java.net/groups/2d/2dawtfiles.html
>>>>>  https://git.openjdk.java.net/skara/blob/master/config/mailinglist/rules/jdk.json
>>>>> $ git skara debug mlrules -v ~/git/skara/config/mailinglist/rules/jdk.json src/java.desktop/unix/native
>>>>> Reading rules file...
>>>>> src/java.desktop/unix/native/common: [2d-dev]
>>>>> src/java.desktop/unix/native/include: 
>>>>> src/java.desktop/unix/native/libawt: [2d-dev]
>>>>> src/java.desktop/unix/native/libawt_headless: [awt-dev]
>>>>> src/java.desktop/unix/native/libawt_xawt: [awt-dev]
>>>>> src/java.desktop/unix/native/libfontmanager: [2d-dev]
>>>>> src/java.desktop/unix/native/libjawt: [awt-dev]
>>>>> src/java.desktop/unix/native/libsplashscreen: [awt-dev]
>>>>> Combined list suggestion: [2d-dev, awt-dev]
>>>>> Final list suggestion is: [2d-dev, awt-dev]
>>>>> $ git skara debug mlrules -d 30 -v ~/git/skara/config/mailinglist/rules/jdk.json .
>>>>> ✅ [2d-dev, awt-dev, serviceability-dev] c32923e0: 8240487: Cleanup whitespace in .cc, .hh, .m, and .mm files
>>>>> ❌ [awt-dev] 7f74c7dd: 8212226: SurfaceManager throws "Invalid Image variant" for MultiResolutionImage (Windows)
>>>>> Suggested lists: [2d-dev, awt-dev]
>>>>> Rules matching unmentioned lists [2d-dev]:
>>>>> src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java - [2d-dev: ^src/java.desktop/share/classes/sun/java2d/]
>>>>>  https://wiki.openjdk.java.net/display/SKARA/Skara#Skara-Workflow
>>>>> The rules are made up of sets of regular expressions for the various mailing lists that are used for reviewing changes going into the JDK. If any rule matches, that mailing list will get a copy of the review request email. For directories containing files that belong to different subsystems, it’s usually a good idea to write the rules in a complementary fashion if possible, so that anything not explicitly mentioned gets a reasonable default. As an example, see these rules for a subset of awt / 2d / i18n files:
>>>>> “awt-dev”: "src/java.desktop/share/classes/sun/awt/(?!font|sunhints|color/|font/|geom/|im/|image/|print/)”
>>>>> “2d-dev”: "src/java.desktop/share/classes/sun/awt/(font|sunhints|color/|font/|geom/|image/|print/)"
>>>>> “I18n-dev”: "src/java.desktop/share/classes/sun/awt/im/“
>>>>> In this example, anything not explicitly indicated as belonging to either 2d-dev or i18-dev will be matched by awt-dev.
More information about the jdk-dev