On Tue, 5 Jan 2016 at 23:46 Ezio Melotti
On Tue, Jan 5, 2016 at 9:18 PM, Brett Cannon
wrote: On Tue, 5 Jan 2016 at 10:48 R. David Murray
wrote:
On Tue, 05 Jan 2016 17:50:53 +0000, Brett Cannon
wrote:
If people are that worried, we could do a daily dump of the data. But unless we can have all GitHub-related comments to an issue not trigger an email update I don't think that's feasible as it would mean two emails for every PR comment; one from GitHub and one from b.p.o.
We can fairly easily have all github-originated comments not trigger an email to the nosy list. We'd just need to add a check for the originating email address to the nosy reaction detector. Even nicer would be to make it a per-user setting (I'd rather get the emails from b.p.o. and disable them on github, myself), but that would be a bit more work.
It would be nice to have *all* of the discussion in one place.
If someone wants to put the work in to make that happen then that's fine by me, but I do consider this a nicety and not a blocker. We haven't had code review comments go into b.p.o on Rietveld either so this isn't that far off from what we have now, especially if we block the transition on adding a link back to the GitHub PR through a web hook.
I agree it's not a blocker, but a better integration between Rietveld and b.p.o was often requested (I tried -- and failed -- to tackle the issue a couple of times). What I want to avoid is having people on github missing b.p.o messages, and people on b.p.o missing PRs/reviews on github. (This was already happening when people were discussing the patch on Rietveld, and others were only looking at b.p.o and/or missing the reviews, both because the email from rietveld ended up in the spam folder, and because there was no way to know from b.p.o if someone posted a review.)
I think it's ok to have discussions on b.p.o and reviews on github (similarly to what we currently do with Rietveld), as long as the everyone gets notified. Discussions that are not strictly related to the code/review shouldn't happen on github.
As for the notifications, I think the best option is: * when a PR is posted, it's also automatically added to the issue (to the list of PRs, similar to the list of patches; generates an email too); * when a review is posted, a new message with a link is added to the issue (this is like a regular message, and generates an email too); * when someone posts a PR/review on github, they will also be added automatically on the nosy list of the issue on b.p.o (so they will receive b.p.o messages); * when a PR/review is posted, people in the issue nosy on b.p.o are NOT added to the PR nosy list (they already get new PRs/reviews notifications from b.p.o).
This means that: * when a PR is posted: * the PR author will get a mail from b.p.o confirming that the PR has been added to the issue if they have a linked account (possibly on top of any email github might send for creating a PR); * people in the issue nosy on b.p.o will get an email that lets them know a PR has been added; * when a review is posted: * the PR author and the reviewers will get two emails (one from github and one from b.p.o if they have a linked account) or one (from github, if they don't); * people in the issue nosy will only get one email (from b.p.o, with a link to the review); * when a message is posted: * the PR author and reviewers will get one email (from b.p.o if they have a linked account) or zero (if they don't); * people in the issue nosy will get one email (from b.p.o);
The "problematic" cases are: * the PR author and the reviewers will receive two emails for reviews if they have a linked account; * the PR author and the reviewers will receive zero emails for b.p.o messages if they don't have a linked account;
The former can be solved if we add a way to disable review emails from b.p.o or if github provides one already (or using email filters). The latter can't be solved unless they link accounts (I don't think b.p.o messages should go to github).
FWIW I would personally prefer to get all emails from b.p.o, either ignoring/filtering duplicated review emails from github, or disabling them from github if possible.
If you agree, this is what needs to be done: 1) automatically add PRs to b.p.o issues;
This is a blocker.
2) automatically add a message on b.p.o when a review is posted on github;
With the way GitHub does reviews, this won't make sense. Each comment on a PR is essentially its own review (as Eric complained about because that means each comment is treated as a separate thing and thus generates its own email). It isn't like with Rietveld where there is a "Review + Email" step to send draft reviews and hence a review is a group of comments.
3) add a "github username" field to b.p.o users to link accounts;
That's a blocker for CLA enforcement.
4) automatically add the PR author (during step 1) and reviewers (during step 2) to the issue nosy list on b.p.o;
I view this as a great nicety but not a blocker.
5) add an option to disable review emails (optional);
This will have to be discussed in connection with the emails per PR comment in case there is a misunderstanding of what a review on GitHub is.
I would consider all these points except the 5th as blockers.
Obviously I don't think they all are, but definitely some. -Brett
Best Regards, Ezio Melotti