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 > > > > >
