[Twisted-Python] github process automation shortcut

[Adi Roiban]

On 27 December 2017 at 02:27, Glyph <glyph at twistedmatrix.com> wrote:
> With more and more ecosystem projects on Github, more or less maintained by
> the same team that maintains Twisted proper, our "review keyword in trac"
> workflow is increasingly awkward.
>
> Github itself has an individualized "review queue" of sorts -
> https://github.com/pulls/review-requested - where you can see open pull
> requests where your review has been requested.  What makes this particularly
> interesting is that you can request review from a team, rather than from an
> individual.
>
> Of course, like everything interesting (ahem, labels), you need repo:write
> permission to do this, so we'd still need a bot.
>
> But!
>
> It appears that the CODEOWNERS file -
> https://help.github.com/articles/about-codeowners/ - can automatically
> request code reviews when pull requests touch relevant code - so we could
> add "* @twisted/$PROJECT-reviewers" to each project on that file and, I
> believe, have a fairly orderly review queue that can be seen in a nice
> unified view by each org member, and by using -reviewers teams, we could
> allow for folks to join or leave any individual repo review responsibility
> that they want.
>
> Actually setting up all the stuff needed to test this requires more time
> than I have on my hands at the moment.  Does anyone else know for sure if
> this works as I imagine it would - specifically, that third party
> contributors would get an automatic review request opened, at least
> initially?
>
> We might still need a bot to re-request reviews, but this seems like a much
> less ad-hoc way of doing this process migration than labels.

I think that we can give it a try as is easy to implement.

----------

I tried CODEOWNERS and for my project (which is multi platforms) and
it does not work for me.

The main reason is that GitHub is "simple" and a soon as you create
the review it will think that you want someone to review it.

Most of the time, the review should be requested after all the tests
are green... and you create the PR to trigger the tests.
With many platforms and with lots of tests, it is probable that a dev
will not run all the test before the push.
The tests might fail, and then you might need another day of work
before requesting the review.

...or you create the PR at end of the day, and you want to do a self
review in the morning.

--------

And then you need the bot anyway for 're-review' ... so why not use
the same bot for explicitly requesting when the review is ready.

-------

You might have a team member just leaving a comment, without any
intention to provide a reject/approve review in the future.
With this action, the team is removed from "Reviewers", hence removed
from the review queue.. and the PR might be lost.
This can be solved by asking team members to just leave simple
comments, and not review comments, but review comments are a nice way
to organize multi-chuck comments.

--------

I am +1 for moving all the tickets from Trac to GitHub so that we will
get rid of the "review" keyword and the tentative ticket flow in Trac.

It will be painful, but I think that it will make it easier to
implement a dev process / workflow.

In an ideal world, we would have plenty of time to document the
workflow, and contributors would have plenty of time to read the dev
documentation,
but I think that we should recognize that everyone is short on time,
so in order to make it easier for everybody to contribute

I don't like that we depend that much on GitHub, but I don't see how
there will be time to keep custom infrastructure.
There is little time to do review, and at this stage I think that is
more important for people to do reviews and work on patches rather
than working on infrastructure.


-- 
Adi Roiban