geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anthony Baker <aba...@pivotal.io>
Subject Re: [DISCUSS] require reviews before merging a PR
Date Fri, 31 May 2019 00:00:42 GMT
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>


> On May 30, 2019, at 4:47 PM, Owen Nichols <onichols@pivotal.io> wrote:
> 
> Some folks have found it really helpful to have the PR author schedule a walk-through
of the changes to give reviewers more context and explain the thinking behind the changes.
> 
> Yes, it’s more work to get reviews.  Apache says (really, mandates) that this must
be part of the process.  I started this discussion to come to consensus on what is expected
of a committer.  Being clear on expectations will help us both to evaluate potential new committers,
and also to hold each other accountable.
> 
> -Owen
> 
>> On May 30, 2019, at 4:25 PM, Bruce Schuchardt <bschuchardt@pivotal.io> wrote:
>> 
>> Maybe your experience is different but I find it hard enough to get even one person
to review my pull requests.  I've resorted to merging minor changes without a review a few
times due to lack of response.
>> 
>> 
>> On 5/30/19 3:51 PM, Owen Nichols wrote:
>>> It seems common for Geode PRs to get merged with only a single green checkmark
in GitHub.
>>> 
>>> According to https://www.apache.org/foundation/voting.html we should not be merging
PRs with fewer than 3 green checkmarks.
>>> 
>>> Consensus is a fundamental value in doing things The Apache Way.  A single +1
is not consensus.  Since we’re currently discussing what it takes to become a committer
and what standards a committer is expected to uphold, it seems like a good time to review
this policy.
>>> 
>>> GitHub can be configured to require N reviews before a commit can be merged.
 Should we enable this feature?
>>> 
>>> -Owen
>>> VOTES ON CODE MODIFICATION <https://www.apache.org/foundation/voting.html#votes-on-code-modification>
>>> For code-modification votes, +1 votes are in favour of the proposal, but -1 votes
are vetos <https://www.apache.org/foundation/voting.html#Veto> and kill the proposal
dead until all vetoers withdraw their -1 votes.
>>> 
>>> Unless a vote has been declared as using lazy consensus <https://www.apache.org/foundation/voting.html#LazyConsensus>
, three +1 votes are required for a code-modification proposal to pass.
>>> 
>>> Whole numbers are recommended for this type of vote, as the opinion being expressed
is Boolean: 'I approve/do not approve of this change.'
>>> 
>>> 
>>> CONSENSUS GAUGING THROUGH SILENCE <https://www.apache.org/foundation/voting.html#LazyConsensus>
>>> An alternative to voting that is sometimes used to measure the acceptability
of something is the concept of lazy consensus <https://www.apache.org/foundation/glossary.html#LazyConsensus>.
>>> 
>>> Lazy consensus is simply an announcement of 'silence gives assent.’ When someone
wants to determine the sense of the community this way, it might do so with a mail message
such as:
>>> "The patch below fixes GEODE-12345; if no-one objects within three days, I'll
assume lazy consensus and commit it."
>>> 
>>> Lazy consensus cannot be used on projects that enforce a review-then-commit <https://www.apache.org/foundation/glossary.html#ReviewThenCommit>
policy.
>>> 
>>> 
>>> 
> 


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message