I am not sure whether this is related to this topic or not, but recently I had to revert one of my commit, before I could just do a revert and push to develop, but now that is blocked. I had to file a PR to revert a change that's causing the pipeline to be red. My question is: do we still need to follow the same process (waiting for one review approval) to revert a commit?
On Thu, Oct 31, 2019 at 10:17 AM Udo Kohlmeyer <u...@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 <br...@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 <jujora...@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 <rhough...@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 <jhu...@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 <onich...@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 <dsm...@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 <doev...@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 <n...@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 < > >>>> alind...@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 < > >>>> onich...@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 > >>> > -- Cheers Jinmei
