Request for assistance: Verify and update mailing list filter rules for jdk/jdk in preparation for Skara transition

[Joe Darcy]

Hello,

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.

Cheers,

-Joe

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 
> 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
>    repository?
> 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
>
>