On Tue, 5 Jan 2016 at 23:46 Ezio Melotti <ezio.melotti@gmail.com> wrote:
On Tue, Jan 5, 2016 at 9:18 PM, Brett Cannon <brett@python.org> wrote:
>
>
> On Tue, 5 Jan 2016 at 10:48 R. David Murray <rdmurray@bitdance.com> wrote:
>>
>> On Tue, 05 Jan 2016 17:50:53 +0000, Brett Cannon <brett@python.org> 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