I am very pro-revert for breaking changes. The Benchmark or Windows tests being a culprit is unfortunate, since the PR pipeline cannot tell us about those, but thats the cost of our excellence :)
On Wed, Apr 29, 2020 at 4:06 PM Owen Nichols <onich...@pivotal.io> wrote: > There are many ways a commit can break the Geode CI pipeline[1] despite > our required PR checks, such as: > - merge a PR that failed some non-required PR checks > - merge a PR that last ran checks awhile ago and now interacts > unexpectedly with other recent changes > - merge a PR that fails Windows tests > - merge a PR that fails Benchmarks tests > > When a bad commit gets through for whatever reason, what should happen > next? For example: > - bring it up on the dev list > - someone should revert the change as soon as possible, allowing the > pipeline to remain green while the issue is addressed > - a reasonable amount of time should be allowed for someone to make a > quick fix, otherwise revert. > - the pipeline should remain broken for as long as it takes to fix, as > long as there is clear communication that someone is working on it > - everyone look the other way and hope it fixes itself > > I’m sure there are other ideas as well. A related question is *who* can > or should revert a bad commit? Only the person that merged the PR? Only > the original author of the PR? The first person to notice the problem? > > What is a reasonable amount of time for this to happen? 2 hours? 2 days? > 2 weeks? Does it depend whether the bad commit is also affecting PR checks > for everyone else vs only depriving downstream consumers of new Geode > -SNAPSHOTs? > > Would you take offense if someone else reverted your commit? Does is make > a difference if your commit is exposing an existing issue (as opposed to > introducing a new bug)? > > Is there a perception that reverts create a lot of extra work? (they > shouldn’t--just start your rework PR with a revert of the revert, then add > additional commits that resolve the issue, so reviewers can easily see what > was missing the first time) > > This is a discussion thread, not a vote. We trust committers to do what’s > best. Would embracing a “anyone can revert, no shame” > revert-first-then-fix culture benefit our community, or is our current > easygoing approach working just fine? > > [1] > https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main
