Bad choice of words. > the author feels confident enough to merge
Was referring to committers, normal author's cannot merge. Thanks & Regards, Amogh Desai On Wed, Aug 6, 2025 at 3:58 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > I believe Pierre suggested that the responsibility lies with the one who > merges it, rather than the author, which makes more sense to me. The author > might not have the permission to merge the backport PR. The person merging > the PR should either merge the backport PR or ping the author to resolve > the conflict, and then merge the PR. > > Quite agree. This is also what we agreed before and what is documented > > https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x > > > The committer who merges the PR is responsible for backporting the PRs > that are 3.0 bugfixes (generally speaking) to v3-X-test (latest active > branch we release bugfixes from). See next chapter to see what kind of > changes we cherry-pick. > > It means that they should create a new PR where the original commit from > main is cherry-picked and take care for resolving conflicts. If the > cherry-pick is too complex, then ask the PR author / start your own PR > against v3-0-test directly with the change. Note: tracking that the PRs > merged as expected is the responsibility of committer who merged the PR. > > Committer may also request from PR author to raise 2 PRs one against main > branch and one against v3-0-test prior to accepting the code change. > > J. > > > > On Wed, Aug 6, 2025 at 12:13 PM Wei Lee <weilee...@gmail.com> wrote: > > > I believe Pierre suggested that the responsibility lies with the one who > > merges it, rather than the author, which makes more sense to me. The > author > > might not have the permission to merge the backport PR. The person > merging > > the PR should either merge the backport PR or ping the author to resolve > > the conflict, and then merge the PR. > > > > Best, > > Wei > > > > > On Aug 5, 2025, at 1:55 PM, Amogh Desai <amoghde...@apache.org> wrote: > > > > > > 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 > > >>>> > > >>>> > > >>> > > >> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > > For additional commands, e-mail: dev-h...@airflow.apache.org > > > > >
