geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Owen Nichols <onich...@pivotal.io>
Subject Re: [DISCUSS] require reviews before merging a PR
Date Thu, 30 May 2019 23:47:56 GMT
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
View raw message