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 > > > > >
