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
