> On Jun 13, 2016, at 8:33 PM, Mark Williams <markrwilli...@gmail.com> wrote:
>
> On Mon, Jun 13, 2016 at 02:35:01PM -0700, Glyph wrote:
>>
>> Thus far all discussion has been on the mailing list. I feel like putting
>> it on the wiki would not be that useful, though; hopefully the discussion
>> will continue for at most another month or two, and it's mostly just a
>> question of coming to consensus about how exactly we're going to use the
>> queue and model discrete review/resubmit events than doing a bunch of work.
>> https://github.com/markrwilliams/txghbot is probably most of what we need
>> already, perhaps with one or two slight tweaks?
>
> I'm the owner of txghbot. I hope it ends up being useful for Twisted!
I strongly suspect that it will be the official solution. Thanks so much for
doing this - the existence of this code is a structural expression of the setup
process which short-circuits me needing to read and process all the developer
documentation ;).
> Despite the stern warning at the top of the README, the process it describes
> should result in a functioning GitHub bot.
Cool. I will set that up soon.
> If you feel adventurous you can write your own webhooks. They're Twisted
> plugins that implement this interface:
>
> https://github.com/markrwilliams/txghbot/blob/master/txghbot/_core.py#L42-L79
>
> I'll claim the lack of any abstraction over the GitHub Webhook API is
> intentional; this remains the authoritative documentation:
>
> https://developer.github.com/webhooks/
>
> Please let me know if there's anything I can do to make txghbot make PRs
> easier for everyone. If it ends up being at all useful I'm happy to transfer
> ownership to the Twisted organization.
The first thing that comes to mind is that you could get Tom Prince to give you
write access to txghbot on PyPI so that this could be 'pip install'ed like
anything else, instead of cut live from master@HEAD :-).
I think that some folks were really over-focused on the whole "closing PRs"
part of the previous discussion, when what I was really trying to get at was
the need for a single, clear "review queue". Something like txghbot is
necessary no matter how we do it because non-(maintainer|reviewer|person with
repo:write access|we need a good standard word for this)s will not be able to
manipulate the labels.
At this point I think closing PRs creates more problems than it solves. In
particular:
It means that people can't push changes to their PRs to experiment with travis
build status, because reviewers will keep closing them, which prevents further
pushes from running in CI.
It means that force-pushing has weird and confusing side-effects (even weirder
and confusinger than usual). I am not a big fan of the rebase/force-push
workflow, but judiciously used (i.e. with interactive rebases) it can help with
splitting up big changes, and it also mitigates Github's atrocious management
of the diff display of long-running branches.
It is definitely perceived as an "abnormal" way of doing reviews on Github.
Point 3 was not enough to dissuade me, but it certainly isn't a point in favor,
and in combination with the other two I don't think it looks good.
So, instead of treating /pulls as our review queue, I think something like
<https://github.com/twisted/twisted/pulls?utf8=✓&q=is%3Apr+is%3Aopen+-label%3A%22awaiting+review+response%22+-status%3Afailure>
will have to do. By subtracting a label from the review query rather than
adding one, we can make it so that if our bot breaks down, contributors can
still get their new submissions reviewed, and in the worst case where it is
down when they want to resubmit, they can work around a broken bot by closing
their "awaiting" PR and opening a new one. (The ability to work around broken
infra is important because I have to assume that things are going to go wrong
with this, it being a distributed system on the public Internet.)
This means that txghbot's responsibility, instead of reopening the PR, will be
to add and remove the 'awaiting review response' label. If you wanted to write
the actual plugin for doing that it might be helpful. And then setting up a
repo where we can play with it to test it out before turning it on for
twisted/twisted :).
-glyph
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
