[petsc-dev] Handling pull requests in a better way

[Richard Tran Mills]

Hi Karl,

Since I know this is in many ways a thankless task, let me thank you for 
taking this on!

I'm a bit late to the discussion, but I want to point out one of the 
issues I've encountered with pull requests: Often a pull request is 
submitted with multiple reviewers listed, and it's sometimes not clear 
how many of the reviewers need to look at it. I've spent some time 
looking over pull requests that I'm a reviewer for, and then when things 
look satisfactory, I mark it "approved". But sometimes my expertise only 
pertains to a portion of the pull request, and at least one of the other 
reviewers needs to look at it. Maybe those reviewers, in turn, figure 
they don't need to look because I've already approved it. And sometimes 
a pull request lists multiple reviewers, simply because any single one 
of several people could probably review the request and then merge it. 
In short: I think that one problem we have with pull requests is that 
it's not exactly clear how many people need to bless a particular 
request before approving and merging to next. On any request where Barry 
is also listed as a reviewer, even if I've thoroughly reviewed something 
and approved, I feel most comfortable waiting for Barry's blessing 
before any "integration" happens. But this is not scalable in Barrys. 
What is a better system?

--Richard

On 3/4/18 10:03 PM, Karl Rupp wrote:
> Hi,
>
> since nobody explicitly objected and since nobody volunteered for the 
> PR integrator role, I'll take over this role for the next month or 
> two. Let's evaluate the process then.
>
> Best regards,
> Karli
>
>
> On 03/01/2018 12:33 PM, Karl Rupp wrote:
>> Dear PETSc folks,
>>
>> I think we can do a better job when it comes to handling pull 
>> requests (PRs). We have several PRs piling up, which after some time 
>> (imho) get merged relatively carelessly instead of reaping the full 
>> benefits of a thorough review.
>>
>> In order to improve the integration of pull requests, I propose to 
>> nominate a PR integrator, who is a-priori responsible for *all* 
>> incoming PRs. The PR integrator is free to delegate a particular PR 
>> integration to someone with the relevant domain-specific knowledge 
>> (e.g. Matt for DMPlex-related things) by appropriate comments on 
>> Bitbucket. In case of delays, the PR integrator is also responsible 
>> for issuing reminders over time (like Barry has done in the past).
>>
>> The idea is to make daily progress with the PRs. One integration step 
>> per day (e.g. testing or merging to next) is presumably enough to 
>> handle the load, whereas things get messy if we let things pile up. 
>> Automated testing may help a bit in the future, but it doesn't 
>> release us from properly reviewing the contributed code.
>>
>> Any objections to my PR integrator proposal? Any volunteers? ;-)
>> If nobody else wants to be the highly esteemed PR integrator, I can 
>> do it. ;-)
>>
>> Best regards,
>> Karli