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] Blocking merge button in PR
Date Mon, 21 Oct 2019 17:14:53 GMT
"Apache asserts that a healthy community is a higher priority than good code. Strong communities
can always rectify problems with their code, whereas an unhealthy community will likely struggle
to maintain a codebase in a sustainable manner.” —apache.org/theapacheway <http://www.apache.org/theapacheway/>

Do the technical solutions we are proposing help make our community tighter, or do they have
the opposite effect?

On one hand, a strong community must trust each other to do the right thing (e.g. vet before
merge, but also monitor after merge).
On the other hand, requiring at least one review might force encourage a little more community
togetherness.

Let’s try this “baby steps” proposal, and follow up in a few months to reflect on this
experiment.

> On Oct 21, 2019, at 10:00 AM, Karen Miller <kmiller@pivotal.io> wrote:
> 
> I have (more than once) committed docs changes for typo fixes without
> review.  I generally label the commits
> with a bold "Commit then Review" message.  But, I am bringing this up as I
> have purposely not followed what
> looks to be a positively-received proposed policy, since I have not gotten
> reviews. If all feel that we need a
> rule for everyone to follow (instead of a guideline that PRs shall have at
> least 1 review), I will follow the rule,
> but I'm a -0 on the process. I get it, and I understand its purpose and
> intent, but I personally prefer to trust that each
> comitter takes personal responsibility for the code they commit WRT waiting
> for tests and/or obtaining reviews.
> Karen
> 
> 
> On Mon, Oct 21, 2019 at 6:24 AM Joris Melchior <jmelchior@pivotal.io> wrote:
> 
>> +1 to the revised approach. I think requiring at least one review is
>> important. More eyes make for better code.
>> 
>> Cheers, Joris.
>> 
>> On Mon, Oct 21, 2019 at 8:11 AM Ju@N <jujoramos@gmail.com> wrote:
>> 
>>> +10 to Naba's proposal, it seems the right thing to do and will help us
>> to
>>> prevent accidentally breaking *develop* while keeping focus on people
>>> instead of processes.
>>> I'd add, however, that the *Merge Pull Request* button should remain
>>> disabled until *all CIs have finished*, and only enable it once the
>> *Build,
>>> Unit, Stress Tests and LGTM are green *(that is, force the committer to
>>> wait at least until all CIs are done)*. *I also agree in that that we
>>> should require *at least one* official approval.
>>> Cheers.
>>> 
>> 
>> 
>> --
>> *Joris Melchior *
>> CF Engineering
>> Pivotal Toronto
>> 416 877 5427
>> 
>> “Programs must be written for people to read, and only incidentally for
>> machines to execute.” – *Hal Abelson*
>> <https://en.wikipedia.org/wiki/Hal_Abelson>
>> 


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