Count me as -0. I have some concerns, but I’m okay trying this and seeing how
it goes.
Dale
From: Owen Nichols
Date: Wednesday, June 9, 2021 at 2:25 PM
To: dev@geode.apache.org
Subject: Re: [DISCUSS] Remove stress-new-test-openjdk11 requirement from PRs
Summarizing this thread so far:
In
Summarizing this thread so far:
In favor of making stress-new non-required:
Kirk
Mark
Myself
In favor of making all PR checks non-required:
Jake
In favor of hashing out a more nuanced balance between making it possible (but
not too easy) to ignore stress-new failures:
Donal
Dale
Maybe that's
If we have consensus, no need to VOTE.
> On Jun 9, 2021, at 12:52 PM, Owen Nichols wrote:
>
> Ok, I'm on board with changing stress-new-test from a required PR check to
> non-required. It's simple, codeowners still have the final say, and it
> neatly avoids the whole topic of overrides.
Ok, I'm on board with changing stress-new-test from a required PR check to
non-required. It's simple, codeowners still have the final say, and it neatly
avoids the whole topic of overrides. Time to take a [VOTE]?
On 6/9/21, 11:42 AM, "Mark Hanson" wrote:
I am agreeing with Kirk's
I am agreeing with Kirk's original desire to remote stress-new-test as a
requirement. All others would remain.
Thanks,
Mark
On 6/9/21, 11:39 AM, "Owen Nichols" wrote:
The concern of holding the discussion via PR comments, rather than the dev
list, is just that many people will be
The concern of holding the discussion via PR comments, rather than the dev
list, is just that many people will be excluded. In the past the Geode
community has viewed overrides as highly exceptional, and perhaps indicative of
a larger problem, so I feel that any override situation is of
I think this process sounds good. My only thought is how this works in github.
I would 100% sign off on this, if it works. I know the way I proposed will
work. I prefer your suggestion Dale, if we have a choice.
Thanks,
Mark
On 6/9/21, 11:22 AM, "Dale Emery" wrote:
Here’s the kind of
Here’s the kind of scenario I’m imagining:
* Code owners and other reviewers review the PR in the usual way (either
before or after the tests finish).
* Stress new test fails, perhaps multiple times.
* The committer investigates and, upon concluding that the failed tests are
not
I don't generally review code until it has passed required tests, because of
the high possibility of re-review. I think that is a pretty common approach ( I
could be wrong). I am a code owner and I think this is the least burden I can
see. I am already reading every line of the changes. Maybe
This would substantially increase the burden on codeowners, because now in
addition to looking at the code itself, they would have to wait for any running
PR checks to complete, as well as remember to come back and look at the PR
after any subsequent commits to make sure the checks are still
Build Update for apache/geode-native
-
Build: #3078
Status: Errored
Duration: 1 min and 43 secs
Commit: 2636726 (master)
Author: Robert Houghton
Message: Re-enable .asf.yaml checks that the release-merge removed. (thanks,
Git)
View the changeset:
I think that process is a bit much. PRs are already a challenge. What do people
think about code owners being the gate. We can accept by custom that there
should be no stress-new-test failures. If there is a failure, a code owner can
request a change or decide to let it go. I think that is
I feel that a traditional [DISCUSS] and [VOTE] on the dev list would be
sufficient and proper to grant approval for an override. Any PR already needs
approval from 1 codeowner per area to merge, so codeowners already have a
little more say because they hold veto power over the PR.
In terms of
I think that there should be room for developers to bypass the stress-new-test
jobs requirement in cases like this, where the test that's failing is an
existing flaky test and that failure is blocking other flaky tests/bugs from
being fixed. Splitting the class up won't save us, because
One other thing to think about is perhaps having a rotating team to deal with
flaky tests, a small team commissioned every three or 6 months to clear out
flaky tests for 1 month. It is good experience
Thanks,
Mark
On 6/9/21, 10:04 AM, "Mark Hanson" wrote:
One other thing is that we
I too like #1 best for now… assuming it’s possible to give code owners this
ability.
Coincidentally, about option #3, II was reading the git release notes just now,
and noticed there’s a new “trailers” feature. It gives git the ability to parse
“key: value” pairs at the end of a commit
One other thing is that we have code owners. If the PR submitter decides to
ignore the stress new test, the code owner can still request a fix. That is
probably a sufficient process.
On 6/9/21, 10:02 AM, "Mark Hanson" wrote:
I agree, I am willing to concede this discussion, as long as we
I agree, I am willing to concede this discussion, as long as we are judicious
and specifically only use it when it is not our test that is failing. It is a
real problem.
Thanks,
Mark
On 6/9/21, 9:46 AM, "Kirk Lund" wrote:
I did think about splitting up dunit tests, but I believe
I do like the suggestions offered up by Dale and would encourage (or even
plead) with my fellow contributors to consider these:
* Allow code owners to override the block, if they can be convinced the
> override is justified.
> * Exclude troublesome tests from stress test runs, either via
I did think about splitting up dunit tests, but I believe
testEventIdOutOfOrderInPartitionRegionSingleHop will remain flaky even if I
move it to a new dunit test. No matter how you dice it up, we end up with a
PR that cannot be merged to develop unless you get lucky after running
stress-new-test
20 matches
Mail list logo
