geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacob Barrett <jbarr...@pivotal.io>
Subject Re: [DISCUSS] require reviews before merging a PR
Date Fri, 31 May 2019 05:07:26 GMT

> On May 30, 2019, at 5:00 PM, Anthony Baker <abaker@pivotal.io> wrote:
> 
> Checkout [1] for some helpful context from the early days.
> 
> Anthony
> 
> [1] https://lists.apache.org/thread.html/108602a14b422abe9c94d46b2c5d02c11a9cbb8b224db08b706c6263@1430991799@%3Cdev.geode.apache.org%3E
<https://lists.apache.org/thread.html/108602a14b422abe9c94d46b2c5d02c11a9cbb8b224db08b706c6263@1430991799@%3Cdev.geode.apache.org%3E>


So it looks like at some point early on we decided on RTC but in practice were are somewhere
in between RTC and CTR. This policy was chosen when we were using Apache git and Review Board.
These policies have not really been updated for the GitHub PR workflow since Apache moved
all git repositories to GitHub. Perhaps we should rectify that.

RTC has proven to be less than efficient as it is difficult to get a single reviewer let alone
3.
CTR would imply that we merged a PR and then get some lazy consensus to keep or revert.

We have been practicing a hybrid in that we post a PR, wait from some lazy consensus and then
commit. We could call it LRTC (lazy review then commit).

I don’t think Apache really mandates any one policy, just that you have policy and stick
to it so that everyone is on even ground.

We can’t expect that for each PR posted that we also babysit and wrangle up 3 members on
the community to give a review. Sure this makes sense for large scale architectural changes,
new features, and some nasty refactoring, but if we define a default of LRTC and the the contributor
can request a RTC policy on a per PR basis.

-Jake



Mime
View raw message