Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make backport.py more flexible for complex pull requests #7260

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

cfm
Copy link
Member

@cfm cfm commented Oct 17, 2024

Status

Ready for review

Description of Changes

The much-loved backport.py script makes 2.5 assumptions that do not hold for complex pull requests like #7143, which we fix here:

  1. There are fewer than 30 commits (the default number of results returned by gh api) in the pull request.
  2. All commits can be merged naïvely via a mass git cherry-pick -x (i.e., there are no merge commits requiring git [cherry-pick → merge] -m).
    1. All of the commits in the pull request should be backported.

Testing

Depending on the state of your local release/2.10.1 branch, you can replicate #7257 with:

$ git branch -D release/2.10.1
$ git checkout -b release/2.10.1 release/2.10.0
$ utils/backport.py 7143 2.10.1 origin --skip 6e3d7bed17537fa8f69f941f21a8eec34e8da113

Deployment

Development-only; no deployment considerations.

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it LGTM; the one change I'm asking for is if can we mention the skipped commits in the PR body.

utils/backport.py Outdated Show resolved Hide resolved
utils/backport.py Show resolved Hide resolved
cfm added a commit that referenced this pull request Oct 18, 2024
@cfm cfm force-pushed the backport-many-but-not-all-the-things branch from 9b3123e to 234d5bf Compare October 18, 2024 00:56
@cfm cfm requested a review from legoktm October 18, 2024 01:01
cfm added a commit that referenced this pull request Nov 15, 2024
@cfm cfm force-pushed the backport-many-but-not-all-the-things branch from 234d5bf to ae00ff7 Compare November 15, 2024 17:39
@cfm cfm added the blocked label Nov 15, 2024
@cfm
Copy link
Member Author

cfm commented Nov 15, 2024

Currently blocked on #7339.

cfm and others added 4 commits November 19, 2024 18:10
For example, Weblate-side merge commits from "develop" require "git
cherry-pick -m" and therefore can't be merged en masse even if we wanted
their contents.
Otherwise only the first 30 commits are returned.
@cfm cfm force-pushed the backport-many-but-not-all-the-things branch from ae00ff7 to 0574c47 Compare November 20, 2024 02:11
@cfm cfm removed the blocked label Nov 20, 2024
@cfm
Copy link
Member Author

cfm commented Nov 20, 2024

Rebased from develop after #7339.

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@legoktm legoktm added this pull request to the merge queue Nov 21, 2024
Merged via the queue into develop with commit 6a875bc Nov 21, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants