On Jul 7, 2018, at 12:36 PM, Tom Most
On Sat, Jul 7, 2018, at 12:08 AM, Glyph wrote:
On Jul 6, 2018, at 4:11 PM, Wim Lewis
I have a minor PR that was reviewed and labeled "cleared to land" a while back, but three months have passed since then and it's still unmerged. Is there something else I need to do to move this along, or did it just fall through the cracks?
Can you include a link to to the issue in question? Probably someone (me?) intended to indicate that it was in the appropriate workflow state for landing, but was waiting for continuous integration to complete, and wandered off and forgot about it. If you can pester a contributor here, or on IRC, someone should be able to land it for you.
Speaking of which, it seems like you've been a fairly stalwart and patient external contributor. Would you like to join the project as a member and merge your own approved stuff to trunk? Mostly, the attendant responsibilities are documented here:
(This used to be the authoritative list - https://twistedmatrix.com/trac/wiki/CommitterCheckList https://twistedmatrix.com/trac/wiki/CommitterCheckList - and I've just updated it to remove some extraneous stuff, but it seems that pervasive cultural understanding of github's workflow have obviated the need for repetitive stern cautions that one should not commit directly to trunk without code review and CI...)
It was tagged "cleared to land", but had build failures unrelated to the changes. I merged forward, pushed to branch, kicked a failing build, and got the green checkmark. It has been merged.
Thanks for driving this to completion, Tom! Much appreciated. For future reference though, when landing stuff on trunk, please use this format exactly: https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechang... https://twistedmatrix.com/trac/wiki/ReviewProcess#Authors:Howtomergethechang... This helps with both automatically closing the associated Trac ticket (I already did that in this case) and properly attributing credit for reviewer points on https://twistedmatrix.com/highscores/ https://twistedmatrix.com/highscores/.
Perhaps we should avoid the "cleared to land" label on PRs from non-committers? I scan through open PRs for ones which require a procedural nudge now and again, but I had not looked at this one as the process appeared to be done with it, and I missed that it was from a non-committer.
Ideally it would be used sparingly, but, the availability of such a process release-valve allows someone to do a review even if they only have time to read the code, and not the time to sit around waiting 40 minutes for some intermittent CI nonsense to shake out. Since this doesn't have a https://twisted.reviews/ https://twisted.reviews/ -like "core gameplay loop" that project members regularly visit, perhaps if you're going to use this label you should always shoot a message to this list as well, to let other contributors know that they should take a look if they have a minute? On that note, it looks like https://github.com/twisted/twisted/pull/1010 https://github.com/twisted/twisted/pull/1010 has suffered the same fate? Any other committers have a minute to land that one? :) -g