[python-committers] Dismiss review if a PR is modified

[Victor Stinner]

2017-07-18 21:21 GMT+02:00 R. David Murray <rdmurray at bitdance.com>:
> On Tue, 18 Jul 2017 12:24:13 +0200, Victor Stinner <victor.stinner at gmail.com> wrote:
>> I'm just not unconfortable with the fact that an approval is kept even
>> if the PR is modified after the review :-/ I would expect a list a
>> notice "changed modified after the review" or something like that. At
>> least, for my own reviews, to remind me to review again.
>
> This could be changed if we have consensus to do so.  Github has a
> setting that will cause existing approvals to be "dismissed" if
> a new commit is pushed.
>
>> Compared to Rietveld, GitHub review tool is as "a good" (not much
>> better, not much worse). Sometimes, I'm lost in reviews: my comments
>> are hidden, I have to unfold many widgets. But it's not that bad. It
>> seems like avoiding rebase works around some of these issues.
>
> I much prefer rietveld over github reviews, but I also much prefer the
> integration between the bug tracker and github over the minimal
> integration we had for rietveld.  Thanks to all the people who made
> that happen, and especially Brett for leading it.

I am in favor of making this change :-)

Victor