geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kl...@apache.org>
Subject Re: [DISCUSS] is overriding a PR check ever justified?
Date Fri, 22 Nov 2019 20:19:24 GMT
Does everyone realize that you just voted NO to what now needs to be done
for "[VOTE] Fix bad-merge of GEODE-7488"?

So right now, if we do not override the PR check, we have no way to fix the
PR checks.

On Fri, Nov 22, 2019 at 11:56 AM Owen Nichols <onichols@pivotal.io> wrote:

> Tallying the votes from this thread, it looks like the majority vote is to
> NEVER allow override even in extreme circumstance.
>
> Naba: -1
> Donal: -1
> Dan: +1
> Udo: +1 (extreme circumstances beyond 99.999999999% case)
> Juan: -1
> Ben: +1 (emergency 'break glass’)
> Mark: -1
>
>
> > On Oct 31, 2019, at 10:17 AM, Udo Kohlmeyer <udo@apache.com> wrote:
> >
> > You are correct... Break glass is a sign that whatever needed to be
> done, was not going to work using the prescribed approach..
> >
> > I see this "break glass" as a special handshake or someone with more
> "authority" needs to be agree with this... but given there is not "someone
> with more authority" in Apache... this would have to be consensus between
> either committers or PMC members.
> >
> > --Udo
> >
> > On 10/31/19 9:58 AM, Mark Hanson wrote:
> >> -1 for "Break glass" approach. Needing a break glass approach is a
> sign. I wonder how hard that would be to make exist. I think our break
> glass approach is that we have someone with the power disable the
> restrictions in Github for the window that we must and then we put it back.
> >>
> >> Thanks,
> >> Mark
> >>
> >>> On Oct 31, 2019, at 9:26 AM, Benjamin Ross <bross@pivotal.io> wrote:
> >>>
> >>> I would agree with udo, +1 to having an emergency 'break glass'
> override
> >>> but -1 to having the ability to do it routinely or for convenience.
> >>>
> >>> The main use case in my mind is for infrastructure related issues that
> >>> block a PR behind unrelated changes which can be really frustrating and
> >>> inconvenient (StressNewTest is a big culprit here) but I'm hoping that
> if
> >>> constant issues with this arise it will lead to fixing the offending
> >>> infrastructure, whether that means changing test itself or the
> architecture
> >>> in which it runs, to make our pipelines more reliable. If we smooth
> over
> >>> PR's that run into issues every time Stress Tests break a test which
> only
> >>> had logging changes (or similar situations) then there's no incentive
> for
> >>> us to improve the Stress Tests job.
> >>>
> >>> Having said all that, being completely without the ability to perform
> an
> >>> emergency override if everything goes sideways and the only way to fix
> it
> >>> is to push a commit which can't get a green pipeline seems like a
> pretty
> >>> good idea to me as long as we all agree on what circumstances qualify
> as an
> >>> emergency.
> >>>
> >>> On Thu, Oct 31, 2019 at 2:43 AM Ju@N <jujoramos@gmail.com> wrote:
> >>>
> >>>> -1 for allowing overrides.
> >>>> If there's an edge case on which it's required, then we could use it
> at the
> >>>> very last resources *if and only if* it's been discussed and approved
> >>>> through the dev list for that particular case.
> >>>> Cheers.
> >>>>
> >>>>
> >>>> On Wed, Oct 30, 2019 at 11:35 PM Robert Houghton <
> rhoughton@pivotal.io>
> >>>> wrote:
> >>>>
> >>>>> Any committer has the 'status' permission on apache/geode.git. Some
> API
> >>>>> tokens have it as well. Its curl + github API reasoning to do this.
> >>>>>
> >>>>> On Wed, Oct 30, 2019 at 2:02 PM Jason Huynh <jhuynh@pivotal.io>
> wrote:
> >>>>>
> >>>>>> If we are going to allow overrides, then maybe what Owen is
> describing
> >>>>>> should occur.  Make a request on the dev list and explain the
> >>>> reasoning.
> >>>>>> I don't think this has been done and a few have already been
> >>>> overridden.
> >>>>>> Also who has the capability to override and knows how.  How
is that
> >>>>>> determined?
> >>>>>>
> >>>>>> On Wed, Oct 30, 2019 at 1:59 PM Owen Nichols <onichols@pivotal.io>
> >>>>> wrote:
> >>>>>>>> How do you override a check, anyway?
> >>>>>>> Much like asking for jira permissions, wiki permissions,
etc, just
> >>>> ask
> >>>>> on
> >>>>>>> the dev list ;)
> >>>>>>>
> >>>>>>> Presumably this type of request would be made as a “last
resort”
> >>>>>> following
> >>>>>>> a dev list discussion wherein all other reasonable options
had been
> >>>>>>> exhausted (reworking or splitting up the PR, pushing empty
commits,
> >>>>>>> rebasing the PR, etc)
> >>>>>>>
> >>>>>>>> On Oct 30, 2019, at 1:42 PM, Dan Smith <dsmith@pivotal.io>
wrote:
> >>>>>>>>
> >>>>>>>> +1 for allowing overrides. I think we should avoid backing
> >>>> ourselves
> >>>>>>> into a
> >>>>>>>> corner where we can't get anything into develop without
talking to
> >>>>>> apache
> >>>>>>>> infra. Some infrastructure things we can't even fix
without
> >>>> pushing a
> >>>>>>>> change develop!
> >>>>>>>>
> >>>>>>>> How do you override a check, anyway?
> >>>>>>>>
> >>>>>>>> -Dan
> >>>>>>>>
> >>>>>>>> On Wed, Oct 30, 2019 at 12:58 PM Donal Evans <doevans@pivotal.io>
> >>>>>> wrote:
> >>>>>>>>> -1 to overriding from me.
> >>>>>>>>>
> >>>>>>>>> The question I have here is what's the rush? Is
anything ever so
> >>>>>>>>> time-sensitive that you can't even wait the 15 minutes
it takes
> >>>> for
> >>>>> it
> >>>>>>> to
> >>>>>>>>> build and run unit tests? If some infrastructure
problem is
> >>>>> preventing
> >>>>>>>>> builds or tests from completing then that should
be fixed before
> >>>> any
> >>>>>> new
> >>>>>>>>> changes are added, otherwise what's the point in
even having the
> >>>> pre
> >>>>>>>>> check-in process?
> >>>>>>>>>
> >>>>>>>>> -Donal
> >>>>>>>>>
> >>>>>>>>> On Wed, Oct 30, 2019 at 11:44 AM Nabarun Nag <nnag@apache.org>
> >>>>> wrote:
> >>>>>>>>>> @Aaron
> >>>>>>>>>> It's okay to wait for at least the build, and
unit tests to
> >>>>> complete,
> >>>>>>> to
> >>>>>>>>>> cover all the bases. [There may have been commits
in between
> >>>> which
> >>>>>> may
> >>>>>>>>>> result in failure because of the revert]  And
it's not hard to
> >>>> get
> >>>>> a
> >>>>>> PR
> >>>>>>>>>> approval.
> >>>>>>>>>>
> >>>>>>>>>> -1 on overriding. If the infrastructure is down,
which is the
> >>>> test
> >>>>>>>>>> framework designed to ensure that we are not
checking in
> unwanted
> >>>>>>> changes
> >>>>>>>>>> into Apache Geode, wait for the infrastructure
to be up, get
> your
> >>>>>>> changes
> >>>>>>>>>> verified, get the review from a fellow committer
and then
> >>>> check-in
> >>>>>> your
> >>>>>>>>>> changes.
> >>>>>>>>>>
> >>>>>>>>>> I still don't understand why will anyone not
wait for unit tests
> >>>>> and
> >>>>>>>>> build
> >>>>>>>>>> to be successful.
> >>>>>>>>>>
> >>>>>>>>>> Regards
> >>>>>>>>>> Nabarun Nag
> >>>>>>>>>>
> >>>>>>>>>> On Wed, Oct 30, 2019 at 11:32 AM Aaron Lindsey
<
> >>>>> alindsey@pivotal.io>
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> One case when it might be acceptable to
overrule a PR check is
> >>>>>>>>> reverting
> >>>>>>>>>> a
> >>>>>>>>>>> commit. Before the branch protection was
enabled, a committer
> >>>>> could
> >>>>>>>>>> revert
> >>>>>>>>>>> a commit without a PR. Now that PRs are
mandatory, we have to
> >>>> wait
> >>>>>> for
> >>>>>>>>>> the
> >>>>>>>>>>> checks to run in order to revert a commit.
Usually we are
> >>>>> reverting
> >>>>>> a
> >>>>>>>>>>> commit because it's causing problems, so
I think overruling the
> >>>> PR
> >>>>>>>>> checks
> >>>>>>>>>>> may be acceptable in that case.
> >>>>>>>>>>>
> >>>>>>>>>>> - Aaron
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On Wed, Oct 30, 2019 at 11:11 AM Owen Nichols
<
> >>>>> onichols@pivotal.io>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>> Our new branch-protection rules can
sometimes lead to
> >>>> unexpected
> >>>>>>>>>>> obstacles
> >>>>>>>>>>>> when infrastructure issues impede the
intended process.
> Should
> >>>>> we
> >>>>>>>>>>> discuss
> >>>>>>>>>>>> such cases as they come up, and should
overruling the result
> >>>> of a
> >>>>>> PR
> >>>>>>>>>>> check
> >>>>>>>>>>>> ever be an option on the table?
> >>>>>>>>>>>>
> >>>>>>>>>>>> -Owen
> >>>>>>>
> >>>>
> >>>> --
> >>>> Ju@N
> >>>>
>
>

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