Request for assistance: Verify and update mailing list filter rules for jdk/jdk in preparation for Skara transition
joe.darcy at oracle.com
Sat Aug 29 00:07:27 UTC 2020
A few general comments on this effort.
There are both type I and type II errors today in terms of RFR mails --
extraneous/off-topic mailing lists being included and mailing lists that
should be added being left off. Both kinds of errors occur today,
fetch-a-different-OpenJDK-mail-alias threads and omitting build-dev for
make changes being common examples, and no doubt both kinds of errors
will occur in the future, hopefully with reduced frequency.
The mapping of
files modified -> set of appropriate mailing lists
should be regular enough to (mostly) capture with automation, letting
developers adjust accordingly when the defaults are not ideal. I think
that information is much better kept in some repo than in a wiki, to
better support tooling. The mapping is not carved in stone and should
expect to be tuned as we start using it in earnest.
On 8/28/2020 7:20 AM, Erik Helin wrote:
> Hi all,
> 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 gray area.
> 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 mailing list.
> 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
> 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
> jdk/jdk repository.
> 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
> be updated.
> 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
> this problem?
> 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!
> Best regards,
> Erik and Robin
More information about the jdk-dev