@Jacob Barrett <jbarr...@pivotal.io> I have some ideas on this. Want to look at in on Thursday with me?
On Sat, Dec 28, 2019 at 5:28 AM Jacob Barrett <jbarr...@pivotal.io> wrote: > Let’s find a way to get the ci, docs, and other directories not effected > by tests out of this testing hold. > > > On Dec 27, 2019, at 3:23 PM, Nabarun Nag <n...@pivotal.io> wrote: > > > > Please maintain the branch protection rules. > > Waiting for reviews and Unit tests to pass does not stifle productivity, > > but prevents us from making mistakes that are detrimental to the entire > > community. If I am not mistaken, we still have pushed code which broke > > builds and regressions. I would suggest not removing but improving / add > > ons to the checks to prevent such issues from happening again. > > > > Also, personally I feel that CI code can separated out of geode code base > > as they have no tests to run and they can circumvent the unit test pass > > criteria. > > > > I would just like to say that fixing tests is all of our responsibility, > > not a particular group of developers. > > > > Regards > > Naba > > > > > > > > > > > > > > > >> On Fri, Dec 27, 2019 at 3:05 PM Jason Huynh <jhu...@pivotal.io> wrote: > >> > >> Just to add more flavor to my previous response... I currently have a PR > >> open that modified a method signature that touched a few WAN tests. It > was > >> a simple change, removing an unused parameter. StressNewTest failed > and I > >> had to spend another day figuring out 10 or so different failures. A > waste > >> of time? Maybe.. At first, I wasn't going to continue, but after > trying a > >> few things, it looks like the tests installed a listener that was > hampering > >> other tests. At the end (soon once it gets reviewed/merged), we end up > >> with a Green PR and hopefully have unblocked others on these specific > tests > >> in the future. > >> > >>> On Fri, Dec 27, 2019 at 2:58 PM Jason Huynh <jhu...@pivotal.io> wrote: > >>> > >>> I feel the frustration at times, but I do also think the ci/pipelines > are > >>> improving, breaking less often. I'm ok with the way things are for the > >>> moment > >>> > >>> On Fri, Dec 27, 2019 at 1:47 PM Owen Nichols <onich...@pivotal.io> > >> wrote: > >>> > >>>> In October we agreed to require at least 1 reviewer and 4 passing PR > >>>> checks before a PR can be merged. Now that we’re tried it for a few > >>>> months, do we like it? > >>>> > >>>> I saw some strong opinions on the dev list recently: > >>>> > >>>>> Changes to the infrastructure to flat out prevent things that should > >> be > >>>> self policing is annoying. This PR review lock we have had already > cost > >> us > >>>> valuable time waiting for PR pipelines to pass that have no relevance > to > >>>> the commit, like CI work. I hate to see process enforced that keeps us > >> from > >>>> getting work done when necessary. > >>>> > >>>> > >>>> and > >>>> > >>>>> I think we're getting more and more bureaucratic in our process and > >>>> that it stifles productivity. I was recently forced to spend three > days > >>>> fixing tests in which I had changed an import statement before they > >> would > >>>> pass stress testing. I'm glad the tests now pass reliably but I was > >> very > >>>> frustrated by the process. > >>>> > >>>> > >>>> Just wondering if others feel the same way. Is it time to make some > >>>> changes? > >>>> > >>>> -Owen > >>> > >>> > >> >
