Send all PR discussion to mailing list

jsm28 commented

2025-10-14 16:39:35 +00:00

Member

When patch review and discussion takes place on a mailing list, subscribers can see all the discussion on the list as it takes place. The same should apply for patch review and discussion that takes place on the forge: reading the list should suffice to follow all the discussion as it goes along.

Each review / discussion comment posted on the forge should automatically go to the same mailing lists as the patch, appropriately threaded.
It's OK to have a short wait to see if there are multiple comments in quick succession on different bits of the patch that can be combined into a single email, but no comments should wait very long to go to the list.
Bot comments with instructions about how to use the forge, and comments such as "/submit" and similar that are intended to drive a bot rather than actually to comment on the patch, should be excluded. CI bot comments should probably be excluded as well, at least when CI succeeds (information about failures might be more useful, though even there it would be a bad idea for lots of CI bots, all failing for a broken PR, each to result in a separate message to the list).
If the patch hasn't been submitted to the list when review comments start being added, adding such comments should trigger sending the version of the patch being commented on to the list. Likewise if people start commenting on diffs from a new version that hasn't yet gone to the list, that version should go to the list, so that those comments make sense in the context of diffs sent to the list.
When comments relate to particular lines of the changes, quote the relevant (unified) diff hunk up to the lines being commented on (I'm concerned here with the plain text email, not any HTML multiparts there might be). The diff hunk header should be in "-p" format, so showing the heuristically determined function name containing the changes being commented on. (I'm not very concerned with whether this has the ability to handle diff= .gitattributes settings, though GCC does have such a setting in .gitattributes and corresponding .git/config setup in contrib/gcc-git-customization.sh.)

When patch review and discussion takes place on a mailing list, subscribers can see all the discussion on the list as it takes place. The same should apply for patch review and discussion that takes place on the forge: reading the list should suffice to follow all the discussion as it goes along. * Each review / discussion comment posted on the forge should automatically go to the same mailing lists as the patch, appropriately threaded. * It's OK to have a short wait to see if there are multiple comments in quick succession on different bits of the patch that can be combined into a single email, but no comments should wait very long to go to the list. * Bot comments with instructions about how to use the forge, and comments such as "/submit" and similar that are intended to drive a bot rather than actually to comment on the patch, should be excluded. CI bot comments should probably be excluded as well, at least when CI succeeds (information about failures might be more useful, though even there it would be a bad idea for lots of CI bots, all failing for a broken PR, each to result in a separate message to the list). * If the patch hasn't been submitted to the list when review comments start being added, adding such comments should trigger sending the version of the patch being commented on to the list. Likewise if people start commenting on diffs from a new version that hasn't yet gone to the list, that version should go to the list, so that those comments make sense in the context of diffs sent to the list. * When comments relate to particular lines of the changes, quote the relevant (unified) diff hunk up to the lines being commented on (I'm concerned here with the plain text email, not any HTML multiparts there might be). The diff hunk header should be in "-p" format, so showing the heuristically determined function name containing the changes being commented on. (I'm not very concerned with whether this has the ability to handle diff= .gitattributes settings, though GCC does have such a setting in .gitattributes and corresponding .git/config setup in contrib/gcc-git-customization.sh.)

👍 1

rdfm added this to the Requested features project

2025-10-15 12:56:55 +00:00

rdfm commented

2026-04-02 06:06:12 +00:00

Owner

I thought about this and here's a slightly more specific description

A "PR rollup" means:
- if the user has force pushed a new version of the patch series, a PR version is created
- if the current PR version has not been posted to the mailing list yet, an up to date email patch series is sent
- messages related to the PR are sent as replies to the cover letter of the latest patch series
  - this includes all comments and reviews
  - this includes requests to review
  - excluding those that were already sent to the mailing list as emails (the list is tracked on a database table)
Messages that must not be sent to the mailing list:
- all messages by the batrachomyomachia bot
- messages by the linaro bot (that do not include a  tag in the comment, which is a convention we can ask the linaro bot to adopt for comments that are meant to be sent to the mailing list)
- comments that include a  tag, which is a convention to allow users to explicitly exclude certain comments from being sent to the mailing list
- comments that contain the /submit or /preview command, since they are meant to trigger an email rather than be included in the email themselves

Workflows

when a PR is opened
- no email is sent
- user gets a welcome comment with guidance on how to use the bot and the available commands (a shorter version)
when someone requests a review on a PR by setting the reviewers button
- "PR rollup" occurs
when someone requests a review on a PR by writing a /submit comment
- "PR rollup" occurs
when someone posts a review on a PR (hits the Finish review button and completes a review, with any review state)
- "PR rollup" occurs
When someone writes a comment that is not filtered by the rules above
- if a patch email has already been sent for the PR, an email is sent to the mailing list with the content of the comment, with In-Reply-To set to the message id of the last cover letter, so that it is grouped in the same thread.
- if no patch email has been sent for the PR, no email is sent to avoid sending messages that are not linked to a patch series. These comments will be included in the email when a patch series is eventually sent.
when a PR is closed without being merged and no patch email has been sent and no messages would be sent
- no email is sent as no interaction happened
when a PR is closed without being merged and at least one patch email has been sent or at least one message would be sent
- a single email is sent to the mailing list notifying about the closure, with In-Reply-To set to the message id of the last cover letter
- the email includes all comments and reviews in sequence

I thought about this and here's a slightly more specific description - A "PR rollup" means: - if the user has force pushed a new version of the patch series, a PR version is created - if the current PR version has not been posted to the mailing list yet, an up to date email patch series is sent - messages related to the PR are sent as replies to the cover letter of the latest patch series - this includes all comments and reviews - this includes requests to review - excluding those that were already sent to the mailing list as emails (the list is tracked on a database table) - Messages that must not be sent to the mailing list: - all messages by the batrachomyomachia bot - messages by the linaro bot (that do not include a `````` tag in the comment, which is a convention we can ask the linaro bot to adopt for comments that are meant to be sent to the mailing list) - comments that include a `````` tag, which is a convention to allow users to explicitly exclude certain comments from being sent to the mailing list - comments that contain the /submit or /preview command, since they are meant to trigger an email rather than be included in the email themselves Workflows - when a PR is opened - no email is sent - user gets a welcome comment with guidance on how to use the bot and the available commands (a shorter version) - when someone requests a review on a PR by setting the reviewers button - "PR rollup" occurs - when someone requests a review on a PR by writing a /submit comment - "PR rollup" occurs - when someone posts a review on a PR (hits the Finish review button and completes a review, with any review state) - "PR rollup" occurs - When someone writes a comment that is not filtered by the rules above - if a patch email has already been sent for the PR, an email is sent to the mailing list with the content of the comment, with In-Reply-To set to the message id of the last cover letter, so that it is grouped in the same thread. - if no patch email has been sent for the PR, no email is sent to avoid sending messages that are not linked to a patch series. These comments will be included in the email when a patch series is eventually sent. - when a PR is closed without being merged and no patch email has been sent and no messages would be sent - no email is sent as no interaction happened - when a PR is closed without being merged and at least one patch email has been sent or at least one message would be sent - a single email is sent to the mailing list notifying about the closure, with In-Reply-To set to the message id of the last cover letter - the email includes all comments and reviews in sequence

rdfm commented

2026-04-14 07:57:49 +00:00

Owner

I wrote a first stab in #36
Here's an example of emails sent https://inbox.sourceware.org/test-list/bmm.hhl9dqjooy.fadmin.d.developer.14.1.0@arm.com/t/#m8e5c8775901ffb3444ba947a0ce94425c75f515d

I wrote a first stab in #36 Here's an example of emails sent https://inbox.sourceware.org/test-list/bmm.hhl9dqjooy.fadmin.d.developer.14.1.0@arm.com/t/#m8e5c8775901ffb3444ba947a0ce94425c75f515d

rdfm commented

2026-04-30 18:01:20 +00:00

Owner

The workflow above has now been enabled for the gcc-patches mailing list.
An example where the bot sends emails as the discussion happens is this email thread that reflects the discussion in pull request 151

The workflow above has now been enabled for the gcc-patches mailing list. An example where the bot sends emails as the discussion happens is [this email thread](https://inbox.sourceware.org/gcc-patches/bmm.hi32i72328.gcc.gcc-TEST.rearnsha.151.5759.CMT@forge-stage.sourceware.org/T/#t) that reflects the discussion in [pull request 151](https://forge.sourceware.org/gcc/gcc-TEST/pulls/151)

rdfm closed this issue

2026-04-30 18:01:20 +00:00

rdfm referenced this issue

2026-04-30 18:29:02 +00:00

Send a message to the mailing list when a reviewer acks a pull request #7

Rows
Columns

Send all PR discussion to mailing list #15