[python-committers] Dismiss review if a PR is modified (original) (raw)

Victor Stinner victor.stinner at gmail.com
Tue Jul 18 16:09:38 EDT 2017

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

More information about the python-committers mailing list