I’m still not interested until there is a solution for all community members to
retrigger failed jobs. Also not excited about not having a way to override if
there is a known issue that prevents a job from going green. My last PR needed
multiple reruns because of a known flakey test with an acti
I’d like to follow up on this discussion from late last year. Six months ago,
Kirk wrote:
> After we get it more consistently GREEN, I would be willing to change my vote
> to +1.
While we’re still not perfect, I have noticed that PR checks go green much more
consistently now than they did six
I'm changing my vote to -1 for disallowing merge until precheck passes.
The reason is that it's rare for me to see a 100% clean precheckin for any
of my PRs. There seems to always be some failure unrelated to my PR. For
example PR #3042 just hit GEODE-6008. If we make the change to disable the
mer
I was responding to Udo's comment:
"Could one not configure the button that the owner of the PR cannot merge
the PR?"
I'm +1 for disallowing merge until precheck passes.
I'm -1 for disallowing the owner of the PR to merge it.
On Fri, Dec 21, 2018 at 9:28 AM Helena Bales wrote:
> Kirk, this cha
Kirk, this change would not require you to get someone to merge it. It
would just require that your PR pass CI before it can be merged.
On Thu, Dec 20, 2018 at 2:38 PM Kirk Lund wrote:
> I have enough trouble just getting other developers to review my PR. I
> don't want to have to struggle to fi
I have enough trouble just getting other developers to review my PR. I
don't want to have to struggle to find someone to merge it for me, too.
On Mon, Nov 19, 2018 at 4:09 PM Udo Kohlmeyer wrote:
> I don't believe "name and shame" is a hammer we should wield, but if we
> have use it... use it wi
+1 to blocking the "merge" button
On Mon, Nov 19, 2018 at 5:09 PM Udo Kohlmeyer wrote:
> I don't believe "name and shame" is a hammer we should wield, but if we
> have use it... use it wisely
>
> Could one not configure the button that the owner of the PR cannot merge
> the PR?
>
> --Udo
>
>
>
I don't believe "name and shame" is a hammer we should wield, but if we
have use it... use it wisely
Could one not configure the button that the owner of the PR cannot merge
the PR?
--Udo
On 11/19/18 16:03, Dan Smith wrote:
Closing the loop on this thread - I don't feel like there was enou
Closing the loop on this thread - I don't feel like there was enough
agreement to go forwards with disabling the merge button, so I'm going to
drop this for now.
I would like to see everyone make sure that they only merge green PRs.
Maybe we can go with a name and shame approach? As in, we shouldn
+1 I like this idea, but I recognize that it will be a challenge when there
is still some flakiness to the pipeline. I think we'd need clear
guidelines on what to do if your PR fails due to something seemingly
unrelated. For instance, we ran into GEODE-5943 (flaky EvictionDUnitTest)
in our last P
Just to clarify, that flaky EvictionDUnitTest is old flaky. The PR to
refactor the test passed all checks, even the repeatTest as well. I had a
closed PR that just touched the "un-refactored" EvictionDUnitTest, it
wouldn't even pass the repeatTest at all.
On Mon, Nov 12, 2018 at 2:04 PM Dan Smith
In that case, why don't we make the rule that the owner of the PR is not
allowed to merge it.
Another committer must then accept and merge it.
--Udo
On 11/12/18 14:03, Dan Smith wrote:
To be clear, I don't think we have an issue of untrustworthy committers
pushing code they know is broken to
To be clear, I don't think we have an issue of untrustworthy committers
pushing code they know is broken to develop.
The problem is that it is all to easy to look at a PR with some failures
and *assume* your code didn't cause the failures and merge it anyway. I
think we should all be at least reru
0
Patrick does make a point. The committers on the project have been voted
in as "responsible, trustworthy and best of breed" and rights and
privileges according to those beliefs have been bestowed.
We should live those words and trust our committers. In the end.. If
there is a "rotten" appl
-1
I really don't think this needs to be codified. If people are merging red
PRs, that is a failing as a responsible developer. Defensive programming
is all well and good, but this seems like it's a bit beyond the pale in
that regard. I foresee it making the correction of a red main pipeline
mu
I'm in favor of this change, but only if we have a way to restart failing
PR builds without being the PR submitter. Any committer should be able to
restart the build. The pipeline is still flaky enough and I want to avoid
the situation where a new contributor is asked repeatedly to push empty
commi
What's the general consensus on flakiness of the pipeline for this purpose?
If there is consensus that it's still too flaky to disable the merge button
on failure, we should probably consider doubling down on that issue again.
It's hard to tell from just looking at the dev pipeline because you cann
I'm in favor of this.
Several times over the years we've made a big push to get precheckin to
reliably only to see rapid degradation due to crappy code being pushed
in the face of precheckin failures. We've just invested another half
year doing it again. Are we going to do things differently
-1
I think there is still plenty of flakiness in the tests that PRs can go red
due to something flaky. Blocking a merge when the issue is known to be
flaky would be disruptive to progress. I think it will also just encourage
someone to use direct pushing to work around false reds. This might lead
+1
> On Nov 9, 2018, at 2:07 PM, Helena Bales wrote:
>
> +1
> As far as the failure rate goes, even if the rate is 25% restarting the
> pipeline usually gives a passing run. And this would make us less likely to
> incorrectly dismiss a failure as not related. If the second build also
> fails wit
+1
As far as the failure rate goes, even if the rate is 25% restarting the
pipeline usually gives a passing run. And this would make us less likely to
incorrectly dismiss a failure as not related. If the second build also
fails with the same failures, those should be investigated and resolved in
so
It looks like the current failure rate (post-PR, including all types of
failures) for DistributedTest is around 25%. Do most people experience similar
failure rates on the *-pr pipeline? I’m specifically wondering about failures
unrelated to your changes.
Anthony
> On Nov 9, 2018, at 12:55
+1
On Fri, Nov 9, 2018 at 12:58 PM Robert Houghton
wrote:
> +1
>
> On Fri, Nov 9, 2018, 12:55 Dan Smith
> > Hi all,
> >
> > Kirks emails reminded me - I think we are at the point now with our PR
> > checks where we should not be merging anything to develop that doesn't
> pass
> > all of the PR
+1 assuming “all of the PR checks” will soon include Java 11
Implementation-wise should be as simple at getting someone with admin
privileges on the geode repo to change the settings for “develop” branch to
"Require status checks to pass before merging."
> On Nov 9, 2018, at 12:58 PM, Robert Ho
+1
On Fri, Nov 9, 2018 at 12:58 PM, Robert Houghton
wrote:
> +1
>
> On Fri, Nov 9, 2018, 12:55 Dan Smith
> > Hi all,
> >
> > Kirks emails reminded me - I think we are at the point now with our PR
> > checks where we should not be merging anything to develop that doesn't
> pass
> > all of the PR
+1
On Fri, Nov 9, 2018, 12:55 Dan Smith Hi all,
>
> Kirks emails reminded me - I think we are at the point now with our PR
> checks where we should not be merging anything to develop that doesn't pass
> all of the PR checks.
>
> I propose we disable the merge button unless a PR is passing all of
Hi all,
Kirks emails reminded me - I think we are at the point now with our PR
checks where we should not be merging anything to develop that doesn't pass
all of the PR checks.
I propose we disable the merge button unless a PR is passing all of the
checks. If we are in agreement I'll follow up wi
27 matches
Mail list logo
