Request for assistance: Verify and update mailing list filter rules for jdk/jdk in preparation for Skara transition
erik.helin at oracle.com
Fri Aug 28 14:20:08 UTC 2020
Thanks for all the feedback, based on this discussion we have been able
to make a number of improvements in this area!
But to begin with, it would probably have been good to clarify the
intended use of this list of rules a bit more clearly: The intent of
these rules is to create a conservative “minimal” initial set of lists
that quite likely would want to be included when sending out the review
request. A good example of this would be the fairly well known rule that
“all Makefile changes should be reviewed on build-dev” which is readily
encoded in this format as “build-dev”: “make/“. However, files that are
shared between several lists and where you select lists based on what
actually changed should not be on here. You may for example have noticed
the absence of the various ProblemList.txt files - they certainly fit
that criteria. But these are the two clear opposites, there are of
course many places in the jdk source tree where this is much more of a
It is also quite likely that the original list that we posted here was
dauntingly verbose - it was our hope that it would be reasonably
straightforward for a subject matter expert to quickly delete a lot of
the “noise”. Based on the change suggestions received so far, it would
appear to not have been that unreasonable: the latest version has
dropped over a hundred lines while still improving accuracy by a fair
bit. Ideally there should probably only be a handful of rules for any
given list, which is already true for most, but the rules for lists like
“core-libs-dev” are certainly not there yet.
We should also mention how the initial list was created - it was *not*
crated manually by us :) Instead we parsed the RFR e-mails for thousands
of commits pushed to the jdk/jdk repository. This allowed us to create a
program that could see the files that were changed by a commit and the
lists that that the RFR e-mail were sent to. We then manually curated
the list, but realized that domain experts would make a better job of
the curation (hence the "request for assitance" e-mail).
We have also validated our approach by comparing the results of the
rules engine for selecting mailing lists with the last 30 days of RFR
e-mails for patches targeting the jdk/jdk repository. So far it seems
like the rules engine perform equally or better than humans at selecting
mailing lists, but it could also be that we are missing some nuances in
RFR e-mails that look like they have forgotten to CC at least one
Still, if you are an experienced contributor, you may not want to have
these suggestions added only to have to replace them manually with the
correct lists. To help with that, we have adjusted the “git pr create”
command-line tool to display the suggestions before the PR is created,
and provide the option to override them. If you create a PR from the
command-line you can now use the --cc flag:
$ git pr create --cc=hotspot-dev,build-dev
Furthermore, the "git pr create" command will now ask the user to verify
the mailing lists before creating the pull request:
$ git pr create
The following mailing lists will be CC:d for the "RFR" e-mail:
- build-dev at openjdk.java.net
- hotspot-dev at openjdk.java.net
Do you want to proceed with this mailing list selection? [Y/n]:
A user can opt-in to always use the inferred mailing lists by speciyfing
--cc=auto. This means that it is now *opt-in* to use the mailing lists
inferred from the patch when creating a PR from the command-line.
When creating a PR from the GitHub web UI, it’s also possible to
override these suggestions by adding a “/cc” command at the end of the
PR body. There is also a window of a few minutes to issue a command to
edit the suggestions after they have been displayed, but before any
email is sent to any list. The semantics of the “/cc” command have also
been changed, to ensure that it always overrides the automatic choices.
We would also like to reply to a few of the additional questions raised:
Q: Shouldn't this file with the rules be placed in the jdk/jdk
A: Yes, we think so too, but we started out by having it in the skara
repository. The Skara tooling does not care where the file resides,
we can easily update the tooling once/if we move the file into the
Q: How will the file be maintained?
A: We imagine that this will be a collective effort once the file has
landed in the jdk/jdk repository. If a contributor makes a large
change that moves a lot of files around, then it seems fitting to
also consider if the rules for choosing mailing lists should perhaps
Q: Why not use the CODEOWNERS file format that GitHub already supports?
A: This is something we considered but rejected for several reasons. The
first and primary reason is that the rule file is not meant to
convey ownership, it is merely stating the mailing lists that should
be notified when certain files change. Secondly we will run into
problems with GitHub _also_ sending e-mails if we use the CODEOWNERS
format, something we would not want. Lastly the rules file also
support specifying mailing lists groups that should be coalesced into
a single mailing list.
Q: Doesn't other OpenJDK projects that already have transitioned have
A: No, for all other OpenJDK projects there is only one "primary"
mailing list. For example skara-dev for Skara, amber-dev for Amber
and so on.
Q: Do we have too many mailing lists?
A: Perhaps, but it is not our intent to propose any changes to the
number of mailing lists we have.
Q: Will the rules engine will be 100% correct every time?
A: No, but neither are humans when they choose mailing lists. The goal
of the rules engine, as stated at the start of this e-mail, is to
choose a reasonable minimal set of mailing lists that a patch should
be sent to. A contributor can always override the choice via the CLI
or use the `/cc` pull request command.
Q: Is the automatic selection of mailing lists opt-in or opt-out?
A: It is currently opt-in if you create a PR from the command-line and
opt-out if you create a PR via a web browser. We believe that many
experienced contributors will use the CLI tools and there have an
easy way to control the selected mailing lists. In constrast many
newcomers are likely to create pull requests via a web browser. As
stated above, there is a "cooldown" period after a PR has been
created when the PR author has time to fine-tune the selected mailing
lists regardless of how the PR was created.
Q: Shouldn't the information about to which mailing list to send a patch
be on the wiki instead?
A: We strongly prefer to have it in a format and place where the
information can be processed by programs. This way we can use the
information to automate and improve upon our workflows.
Again, thanks everyone for the great feedback and to everyone who helped
out tweaking the rules!
Erik and Robin
On 8/27/20 12: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