[Twisted-Python] overview: new review queue venue

Daniel Sank sank.daniel at gmail.com
Sun May 22 00:10:36 MDT 2016


Dear all,

While I'm just a lurker, having used Twisted and Github for some time in a
moderately sized team I would like to offer a couple comments:

> This is exactly why issues and PRs should be separated.  If you only have
one
> artifact - the PR - to represent both the issue and the potential
solution, then
> you can't get rid of the potential solution (reject the PR) without also
getting rid
> of the description of the problem.  Github already automatically shows
> anywhere that your PR or issue is mentioned, so all you need to do to say
"how
> about this approach" is to put the words "fixes #NNN" in your PR
description.
> As a bonus, that will make it automatically close the issue when the PR is
> merged.
​
I cannot agree with this enough.

> 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.

This is a real problem for my team, and I think it happens because we don't
respect the value of issues enough. We have two kinds of branches

1) Those which associate to a specific issue. If issue #123 is about fixing
the quizzwopper, you can make a branch called 123-fix-quizzwopper. This is
a Good Thing.

2) Those which associate to a person. We name these things like
u/danielsank/fix-quizzwopper. The point of that originally was to make
things more "lightweight" and "dynamic" by allowing folks to make a branch
and write some code asap. While this does often work, it also means that we
have a mountain of near unidentifiable branches with no obvious place to
look for that branch's motivation/history/status/whatever.

Type 1 branches have the extra advantage that they're more likely to
attract useful input from people at the right time, i.e. before the code is
written. It's very frustrating to have a PR representing many hours of
someone's work come in only to realize that you want to respond with "Oh
for the love of all that is good in this world please don't do that".
Discussion in an issue helps avoid that.

> 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.

Again, agreed.

> One thing that pull requests encourage is discussion

It's better for that to happen in issues. I have written code, submitted it
as a PR, realized (via feedback and otherwise) that the code is total crap,
deleted the branch and closed the PR, and then gone back to the issue to
re-evaluate my life choices. This is really useful.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20160521/233bdc81/attachment-0002.html>


More information about the Twisted-Python mailing list