FYI, I changed my mind about auto-merging.
In the last couple of days I merged many PRs quickly when they were still
running because I "knew" they were ok. It would be huge impact if I had to
wait for all of them to succeed. I think now that the marginal improvement
in "focus" when reviewing the P
> That said, I want to push back on the framing of some feedback as
“negative.” I really appreciate the folks who raised concerns, those
perspectives are vital to making the project stronger and more inclusive.
I think what I wanted to say is that I think we should all exercise
empathy. It's super
That's what I was saying. But welcoming discussion or feedback and then
terming it negative is a no-go, Jarek.
On Thu, 1 May 2025 at 15:07, Jarek Potiuk wrote:
> I propose - let's not be defensive or offensive, but try to hear each other
> and improve things in the future :).
>
> On Thu, May 1
Could you show me the email where I start with that? Check
https://lists.apache.org/thread/ofxnb3k5vjqsdlf8wpp7td4n1fjrmmoq again
On Thu, 1 May 2025 at 15:05, Jarek Potiuk wrote:
> Yeah. It more about the communication. "-1" is - literally - by definition
> "negative" when you start your message
I propose - let's not be defensive or offensive, but try to hear each other
and improve things in the future :).
On Thu, May 1, 2025 at 11:35 AM Kaxil Naik wrote:
> >But I would personally love to see more "yes, but" than "no".
>
> Saying "would love to hear what you think" in the original messa
Yeah. It more about the communication. "-1" is - literally - by definition
"negative" when you start your message with "I am strongly -1 on that".
There is no further explanation given that changes that perception.
On Thu, May 1, 2025 at 11:32 AM Kaxil Naik wrote:
> "-1" was backed with rational
>But I would personally love to see more "yes, but" than "no".
Saying "would love to hear what you think" in the original message followed
by terming "negative" is not the way to collaborate.
On Thu, 1 May 2025 at 15:01, Kaxil Naik wrote:
> "-1" was backed with rationale discussion and no one h
"-1" was backed with rationale discussion and no one has said it was a bad
idea but you have said it is "negative".
On Thu, 1 May 2025 at 14:59, Jarek Potiuk wrote:
> > That said, I want to push back on the framing of some feedback as
> “negative.” I really appreciate the folks who raised concer
Thanks for the energy and initiative here.
That said, I want to push back on the framing of some feedback as
“negative.” I really appreciate the folks who raised concerns, those
perspectives are vital to making the project stronger and more inclusive.
Let’s make sure we continue to welcome both e
I really like Jens's line of thought. Rather than focusing on the negative
side, try to figure out a way to make it work :). That's very inspiring.
I think the original slack proposal of Jens was rather brittle, but it made
me think that we can actually implement what we need rather quickly.
Foll
As there was a call for more opinions. Here I am :-D
I understand both positions. As I like AutoMerge very much I am not
giving up :-D I'd like to keep it.
I think there is still an option in between. Maybe need to draft a bit
of thoughts but I think we could build something still around the
I am also in line with Jens. The auto-merge feature works well in keeping
the CI pipeline green before merging.
However, there are situations where we need to merge changes quickly.
Instead of using an
escape route (which I’m not a fan of, as it carries risks if something goes
wrong—even though we
> Regarding what we need, though, I don't think we want to disable branch
protection or allow overriding it. If that happens, PRs can be merged to
main without any approval. What we really need if we enable auto-merge is
some way to bypass and manually merge the PR without waiting for all checks
to
A comment here - and we would not have learned all that without us trying
it first ! So - in retrospect - I am glad I did it, and we should do it
more often IMHO to try and see and then go back with learnings (but yes, it
should have been discussed at devlist if even briefly). That discussion
woul
Exactly!
> That is an interesting point - yes, there is no way to say "only bypass
> some branch protection features" - it's either all or none.
Correct, good reminder, but the question is what do we want? I don't think
we want that. So just because one flaw exists (which would hopefully be
fix
Hey all,
As a concept, it can save us and make the main stable for the things
slipping from eyes. I agree and experincing myself as well hard time asking
rebase on unrelated faluires. I haven't used the `escape hatch` myself. I
always tried to make it green. It is indeed a bit time consuming at st
Regarding what we need, though, I don't think we want to disable branch
protection or allow overriding it. If that happens, PRs can be merged to
main without any approval. What we really need if we enable auto-merge is
some way to bypass and manually merge the PR without waiting for all checks
to b
And yes - apologies once again i have not checked and discussed it before.
We SHOULD discuss it in devlist - and I **should** hold myself accountable
to that similarly as I hold others, and I hope - similarly - when we see
other important things not being discussed here - people will not complain
a
> I also tend to be -1 on this.
So If there are also few others who have opinions here - (other than yes, I
really thought we discussed it in devlist before - but it seems it was only
on slack where we discussed it at length) - from what I see - clearly that
the "escape hatch" does not cut it and
> Why was a workflow change that impacts every. single. PR. not discussed
in this list before turning it on? I can’t think of a more impactful change
to committers.
My mistake. It should be - similar as all the other things we discussed
then. I actually thought we discussed it. I will do better ne
Maybe I need to get used to the `escape hatch` (which I don't really like
tbh), but I've been having a really hard time not being able to merge
things with unrelated CI failure:
- Asking people to rebase again numerous time, until CI gets green, for
instance https://github.com/apache/airflow/pull/
Why was a workflow change that impacts every. single. PR. not discussed in this
list before turning it on? I can’t think of a more impactful change to
committers.
Just last month weren’t you saying we needed to have more discussion about
impactful change on this list?
> On 30 Apr 2025, at 1
Yeah, And I would encourage everyone to try it and provide feedback while
it is enabled.
So far we identified a few things (and fix them) that made it borderline
unusable (bug in workflow for UI-only changes, GitHub starting to 429-us
with too many requests, and the mysterious "hanging" of the lat
Forgot to note an additional point in Summary: If we find anything blocking
us in that period, we will merge
https://github.com/apache/airflow/pull/50009 to disable auto-merge.
On Wed, 30 Apr 2025 at 14:26, Kaxil Naik wrote:
> Jarek & I discussed it on Slack in #internal-airflow-ci-cd. Summary b
Jarek & I discussed it on Slack in #internal-airflow-ci-cd. Summary below:
I have a PR to disable it: https://github.com/apache/airflow/pull/50009.
However, given that many countries will be on holiday on May 1 due to
Labour Day, and some teething issues were fixed yesterday, we will let it
run fo
Whoops yeah.
>Yep. Because it did not have all conversations resolved. We also have
"require resolved conversation" as one of the branch protection conditions.
I resolved the conversation and it got merged automatically.
Let's adapt it when ready though, I don't see any urgency of getting
enablin
On Wed, Apr 30, 2025 at 7:55 AM Kaxil Naik wrote:
> To the point that the original PR is still not merged even after I had
> re-triggered the failed tests yesterday:
> https://github.com/apache/airflow/pull/49727
>
>
Yep. Because it did not have all conversations resolved. We also have
"require r
To the point that the original PR is still not merged even after I had
re-triggered the failed tests yesterday:
https://github.com/apache/airflow/pull/49727
On Wed, 30 Apr 2025 at 11:20, Kaxil Naik wrote:
> The gitbox escape hatch isn't it though -- if we are to allow that why not
> just allow
The gitbox escape hatch isn't it though -- if we are to allow that why not
just allow people to merge it directly from github that to go via an
"escape hatch".
I am -1 on this auto-merge feature
On Wed, 30 Apr 2025 at 11:18, Kaxil Naik wrote:
> That’s not a single person, it impacts the commit
That’s not a single person, it impacts the committers and the PR author
involved too. I don’t see how team productivity soars here.
On Wed, 30 Apr 2025 at 02:39, Jarek Potiuk wrote:
> But yes, I also miss the previous "merge because I think it's safe"
> workflow.
>
> I badly miss it. Personally,
But yes, I also miss the previous "merge because I think it's safe"
workflow.
I badly miss it. Personally, It hurts my productivity.
But I think the "require status check" to be green is great for "team
productivity". Usually when single person is impacted more than team in
general, it's worse fo
Just to add comment:
a) there was some instability of "celery/boto" hanging tests today that is
rather difficult to address - but we worked around it by just removing
"special-tests" from pre-requisite of merging
b) GitHub today (like literally today!) started to be picky on "too many
requests" -
Similar experience as Elad, I am in favor of disabling it tbh. For example,
https://github.com/apache/airflow/pull/49727 has a failing test as below --
which is not an issue, and test passes locally so I would want to merge it
but I can't.
FAILED
helm-tests/tests/helm_tests/airflow_aux/test_basic_
On Tue, Apr 29, 2025 at 1:46 PM Elad Kalif wrote:
> Thanks for that Jarek!
> I find the lack of ability to merge PRs fast very limiting but it might be
> just something to get used to.
>
Indeed. I also see it, but also I got a few manually pushed "must fix
quickly" to gitbox, and actually I find
Thanks for that Jarek!
I find the lack of ability to merge PRs fast very limiting but it might be
just something to get used to.
On Tue, Apr 29, 2025 at 6:18 AM Amogh Desai
wrote:
> I gave it another try and I am convinced that it is helpful now.
>
> > (if we enable it for v3-0-test) - potenti
I gave it another try and I am convinced that it is helpful now.
> (if we enable it for v3-0-test) - potentially we could disable the
"draft" option for backport PRs - even if workflows are not going to be
started those backport PRs will have this "required status check" that will
prevent accide
BTW. The nice side-effect of it is that with this "required" check, we
should not have any problems any more with merging PRs that had:
* bug in workflow resulting in no workflow run at ll
* new contributors that have PRS where workflow are waiting for approval
* (if we enable it for v3-0-test) -
> That was a temporary issue - it is green now. I needed to figure out how
to
name checks properly, and it turned out that "Tests / Finalize tests /
Summarize Warnings" is named (for whatever reason) as "Finalize tests /
Summarize Warnings" when run as part of the composite "umbrella" workflow
"Tes
Hi all,
thanks for tuning on auto-merge.
Agree that there might be a residual risk that pipeline goes green "too
early" and approval is there... but I think we need to use carefully.
I just did my first auto merge and carefully watched... all looks good.
If it helps in only 50% of cases to r
On Sat, Apr 26, 2025 at 3:02 PM Amogh Desai
wrote:
> I am not sure if I like this feature a lot with my recent experience with
> this PR: https://github.com/apache/airflow/pull/49692.
>
> On the PR status, it mentions "finalize tests" is waiting to report status
> but when i describe the tests, I
I am not sure if I like this feature a lot with my recent experience with
this PR: https://github.com/apache/airflow/pull/49692.
On the PR status, it mentions "finalize tests" is waiting to report status
but when i describe the tests, I see that task has completed.
It is quite confusing to me wit
Hello everyone,
With some initial teething problems we've enabled an "experimental" feature
of "auto-merging" PRs in our repo. It should potentially help with "focus"
of maintainers, because they will not have to come-back to the PRs to merge
it once they enabled auto-merge for them.
It works in
42 matches
Mail list logo
