> On May 21, 2016, at 3:57 PM, Terry Jones <te...@jon.es> wrote:
>
> Hi Glyph
>
> On Sun, May 22, 2016 at 12:12 AM, Glyph <gl...@twistedmatrix.com
> <mailto:gl...@twistedmatrix.com>> wrote:
> Reviewing: This is the potentially slightly odd part. I believe a review
> that doesn't result in acceptance should close the PR.
>
> This feels wrong to me. I find github pull requests very useful, in ways that
> it sounds like your suggestion would cut off - if I understand you right. Do
> you mean that the one person who decides to do the formal review would set
> themself as the "assignee" and, following their review, close the ticket if
> the change isn't up to standard?
The "assignee" field isn't all that useful; really, it should be set to the
submitter, but github has weird rules about who can be assigned (last I
checked, only contributors).
> And in the meantime others (who are not the assignee) would be free to
> comment at will, with the pr staying open?
Drive-by comments on a PR are sometimes helpful, but should be used sparingly.
Mostly, discussion should happen on issues, not PRs. A PR is a suggested
resolution to a problem, and we might reject one solution, but an issue should
describe the problem itself.
> If so, that's good, but I still don't like to think that all discussion
> around a pr then gets shut down on the say so of the single reviewer.
The PR gets closed, not deleted. People can still comment if they like.
> One thing that pull requests encourage is discussion - sometimes it's a way
> to ask for input on how to proceed, sometimes it's just people chipping in a
> little bit with a code optimization, sometimes people saying "if you do this
> then this other thing will happen" etc. They help people learn, to easily
> make small contributions (that can lead to larger ones), to see what's going
> on, to judge the health of a project, etc. I like how dynamic and lightweight
> that process can be in a Github pull request. It feels to me that closing
> pull requests is a step in the other direction - back towards something that
> feels more monolithic and more old fashioned.
You see "dynamic and lightweight", I see "unfocused and noisy" :).
One of the things I still don't like about github is the tendency for projects
to build up huge piles of open PRs, which nobody wants to reject because
they're maybe possibly useful, but they won't accept because they don't meet
quality standards. Contributors don't get clear feedback about whether they're
expected to do more or whether the project will take it over, and people
frequently get mad about their stuff not being merged. I think by closing PRs
that we aren't going to merge as-is we can send a much clearer signal about
what the project is taking on versus what it expects external contributors to
do more work on.
> As usual, I'm sure that there's absolutely nothing I can say on any tech
> subject that you don't know already :-) But those are my thoughts...
While I disagree, I also think that this is a very common perception, and
another one of the functions of the github bot could perform would be a form
comment after the PR is closed, to always submit a form comment to explain what
closing the PR means and how to reopen it.
-glyph
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
