Good point Pierre, yes it will anyways be reviewed when it's going to be ported over to v3-0-stable.
I think the bottom line here is that if it's a simple PR with no conflicts / the author feels confident enough to merge, they should. Thanks & Regards, Amogh Desai On Mon, Aug 4, 2025 at 5:31 PM Pierre Jeambrun <pierrejb...@gmail.com> wrote: > Hello, > > I also considered that this is the responsibility of the person merging the > PR to make sure that it is backported correctly if applicable. (PR opened > and merged). I always try to fill in when I see that some PR is missing the > backport, but it's harder when this is a PR we didn't follow along > ourselves. > > I do not wait for approval before merging the backport PR because I > consider that this is basically preliminary work for the release and we do > not wait for approval when doing this step of the release process (cherry > picking to the test branch). It will be reviewed later on when merging to > the stable branch. > > Pierre > > On Mon, Aug 4, 2025 at 11:32 AM Jarek Potiuk <ja...@potiuk.com> wrote: > > > > Just want to make sure. Should we merge our own backport PR when it’s > > ready? I feel a bit uncomfortable merging it, as it’s technically my > code. > > I still requested others to review it again in the past few occurrences. > > > > As I see it - yes we can merge them, but I think (as usual) - it > depends > > :). > > > > If this is "obvious" that things should be backported (we have quite > > detailed rules for that > > > > > https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#what-do-we-backport-to-v3-x-test-branch > > ) then it's perfectly fine. The code is **just** backported (ideally with > > just trivial conflict resolution or even "clean" cherry-pick) and the > code > > itself has already been approved by others in order to be merged to main. > > > > One of the reasons we clarified (few times) what are the general rules > for > > back-porting was that we can do it without unnecessary "waiting" - it's > > like a 'blanket approval" - set of guidelines that we agreed to "what we > > should cherry-pick" and if committer assesses that their commit falls > into > > those criteria, this is fine to merge (but I think "green" status should > > also be pre-requisite). > > > > Additionally - this is "v3-0-test" branch and before we turn it into a > > "stable" branch, we have a chance (and some of us like Jens and myself > do) > > to take a look and maybe not review every single line of code, but walk > > through all the commits and see if there is anything raising a "yellow" > or > > "red" flag. The current one about it is here > > https://github.com/apache/airflow/pull/54045 and before approving it and > > final merge several of us should take a look if there is anything > > suspicious - and we have a chance to remove some of those then (even now > > Ash removed my commit that broke v3-0-stable because I merged it too > > quickly as I have not realised there is another commit needed to be > > cherry-picked as well). > > > > Many ASF projects even do it by default - there are two ways of > reviewing: > > "RTC" (Review then Commit) and "CTR" (Commit then Review) - we are using > > RTC for main and "stable" branches but for `test` branches we use CTR > > effectively (mostly because we have the rules and the code committed > there > > is already reviewed). But some ASF projects use CTR for everything and > they > > review all commits "after" they were merged to main. > > > > But of course when there are bigger or possibly controversial or > > "more-conflict-y" changes - then by all means we should ask others before > > merging to test - nothing wrong with that. > > > > J > > > > > > On Mon, Aug 4, 2025 at 8:17 AM Wei Lee <weilee...@gmail.com> wrote: > > > > > Just want to make sure. Should we merge our own backport PR when it’s > > > ready? I feel a bit uncomfortable merging it, as it’s technically my > > code. > > > I still requested others to review it again in the past few > occurrences. > > > > > > Best, > > > Wei > > > > > > > On Aug 4, 2025, at 2:03 PM, Amogh Desai <amoghde...@apache.org> > wrote: > > > > > > > > Yeah, it's easy to ignore messages from the cherry picker sometimes > > > > assuming that adding PRs to the release milestone is good enough, but > > > > more often than not the PR authors should ensure that their PRs land > in > > > the > > > > right set of branches to avoid any surprises. > > > > > > > > Thanks & Regards, > > > > Amogh Desai > > > > > > > > > > > > On Sun, Aug 3, 2025 at 5:58 PM Jarek Potiuk <ja...@potiuk.com> > wrote: > > > > > > > >> Hello here, > > > >> > > > >> I think we (including myself) dropped the ball a bit when > > cherry-picking > > > >> some of the last weeks PR - resulting in unnecessary release > managers > > > >> (Ash's) work when he had to fix some issues in v3-0-stable and I had > > to > > > >> also make the `v3-0-test` green to generate constraints. > > > >> > > > >> I think there are a few things we should pay attention to that are > > > >> relatively quickly merging cherry-picker's PR when they get green > (or > > > >> manually doing it if they cannot be done due to conflicts). I think > > (not > > > >> sure) it's mostly on the commiter who merges PRs - they should see > the > > > >> follow-up comments from the "cherry-picker" and either mark the PR > as > > > >> "Ready for review" and merge it when it gets green., or manually do > > > >> cherry-picking and resolving conflicts (and also merging it when it > > gets > > > >> green). > > > >> > > > >> This is quite important - regarding the sequence of merging the - > and > > > doing > > > >> it "relatively quickly" - because there might be other PRs that > might > > > >> depend on the already cherry-picked PRs - so they might be easier to > > be > > > >> automatically done by cherry-picker. > > > >> > > > >> Also - waiting for the PR to get green (and either solving or asking > > for > > > >> help if it's not) is important because it saves the release manager > a > > > lot > > > >> of work on figuring out why v3-0-test and v3-0-stable is "red". And > it > > > >> makes it easier for the others who cherry-pick PRs after because it > > > avoids > > > >> their "red" cherry-pick that they have no idea where it comes from. > > > >> > > > >> I personally dropped a ball on it over the last few days / weeks > and I > > > am > > > >> as guilty as anyone else with merging things too quickly - but also > I > > > saw a > > > >> number of cherry-pick PRs that looked abandoned for a few days even > if > > > all > > > >> they needed were to make them "Ready for Review". > > > >> > > > >> Maybe a good idea to pay a bit more attention to those when we are > > > merging > > > >> PR that is supposed to be backported. > > > >> > > > >> J. > > > >> > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > > > For additional commands, e-mail: dev-h...@airflow.apache.org > > > > > > > > >
