[Twisted-Python] overview: new review queue venue

Amber "Hawkie" Brown hawkowl at atleastfornow.net
Sun May 22 00:08:03 MDT 2016


> On 22 May 2016, at 06:12, Glyph <glyph at twistedmatrix.com> wrote:
> 
> It is therefore tempting to map it into GitHub via labels and webhooks and bot workflows.  However, I think a better mapping would be this:
> 
> 	• Proposing: Just open a pull request.  Any open pull request should be treated as part of the queue.
> 	• Accepting: A committer pushes the big green button; this
> 	• Reviewing: This is the potentially slightly odd part.  I believe a review that doesn't result in acceptance should close the PR.  We need to be careful to always include some text that explains that closing a PR does not mean that the change is rejected, just that the submitter must re-submit.  Initially this would just mean opening a new PR, but Mark Williams is working on a bot to re-open pull requests when a submitter posts a "please review" comment: https://github.com/markrwilliams/txghbot
> 	• Responding: A submitter can open a new PR, or, once we start running txghbot, reopen their closed PR.
> 	• Viewing: https://github.com/twisted/twisted/pulls?utf8=✓&q=is%3Apr+is%3Aopen+-status%3Afailed

I think this is reasonable. I believe txghbot is in a place where we can start using it for just that.

> The one thing that this workflow is missing from trac is a convenient way for committers, having eyeballed a patch for any obvious malware, to send it to the buildbots.
> 
> We could also potentially just replace our buildbot build farm with a combination of appveyor and travis-ci; this would remove FreeBSD from our list of supported platforms, as well as eliminating a diversity of kernel integrations.  However, for the stuff that doesn't work in containers (mostly inotify) we could run one builder on non-container-based infrastructure, and for everything else (integrating with different system toolchains) we can test using Docker: https://docs.travis-ci.com/user/docker/.  I am very much on the fence about this, since I don't want to move backwards in terms of our test coverage, but this would accelerate the contribution process so much that it's probably worth discussing.

I don't think that this will do us any good. Having travis/appveyor to be a first line of review (giving a quick "builds fail/builds probably pass) so that contributors get an immediate "does this have a chance of being merged" is a good thing; but there is certainly value in having the various different platforms. All we need is a bit more tooling around it (and I think txghbot is a good starting place for that).

We also need to look into some extra tooling around tox; mainly around the ratcheting quality checkers -- some form of fetching the latest build from buildbot and downloading it locally to compare, or something.

> 10 years ago or so, we would routinely discover kernel bugs in our integration test harness and they would be different on different platforms.  But today's platform realities are considerably less harsh, since there are a lot more entities in the ecosystem that have taken responsibility for testing that layer of the stack; I couldn't find anything since 2008 or so where we really saw a difference between Fedora and Ubuntu at the kernel level, for example.

The issue in 2016 is not actually the kernel; but OpenSSL and OpenSSH, among other system libraries. Every new version of Fedora has been red on the buildbots for this reason; OpenSSL changed, and we needed to fix our use of it. Worth also noting is that Travis is so horrendously behind in all things Python+Ubuntu (do they even have a current PyPy yet?) that we're not actually testing the platforms people are *using*, which is something I think is valuable that the current system gives us.

> 
> Thoughts?
> 
> -glyph

- Amber
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: </pipermail/twisted-python/attachments/20160522/8e1a21d7/attachment.sig>


More information about the Twisted-Python mailing list