geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ernest Burghardt <eburgha...@pivotal.io>
Subject Re: [DISCUSS] Blocking merge button in PR
Date Mon, 21 Oct 2019 17:11:59 GMT
+1 to enacting this immediately... just this weekend a PR was merged with
failures on all of the following:
* concourse-ci/DistributedTestOpenJDK11 * Concourse CI build failure
* concourse-ci/UnitTestOpenJDK11 * Concourse CI build failure
* concourse-ci/UnitTestOpenJDK8 * Concourse CI build failure
* concourse-ci/UpgradeTestOpenJDK11 * Concourse CI build failure


Thanks,
EB

On Mon, Oct 21, 2019 at 10:01 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