[riot-notifications] [RIOT-OS/RIOT] dist/tools: Add script for backporting PR's (#8968)

Martine Lenders notifications at github.com
Fri Jan 25 18:06:00 CET 2019


miri64 commented on this pull request.

Not sure if some of this stuff can be done in a later refactoring step. So I'm not blocking.

> @@ -0,0 +1,77 @@
+Pull requests backport script
+=============================
+
+This script provides functionality to easily backport a merged pull request to
+a release branch.
+
+It relies of having a `github` API token stored in `~/.riotgithubtoken` by

```suggestion
It relies on having a `github` API token stored in `~/.riotgithubtoken` by
```

> +request. It then looks for the last release branch.
+A temporary git `worktree` with a new branch is created based on this release
+branch. All commits from the pull request are then cherry-picked into this
+branch which is then pushed to `origin`.
+It then creates a new pull request on `github` with a reference to the original
+pull request. It optionally puts a comment under the original pull request to
+the new backport pull request.
+
+
+
+Usage
+-----
+
+Most common usage would be to run:
+
+    backport_pr.py --comment PR_NUMBER

If it's the most common use-case, why is '--comment' not activated by default?

> +                        help="File containing github token")
+    parser.add_argument("-c", "--comment", action="store_true",
+                        help="Put a comment with a reference under"
+                             "the original PR")
+    parser.add_argument("-n", "--noop", action="store_true",
+                        help="Limited noop mode, creates branch, but doesn't"
+                             "push and create the PR")
+    parser.add_argument("-r", "--release-branch", type=str,
+                        help="Base the backport on this branch, "
+                             "default is the latest")
+    parser.add_argument("--backport-branch-fmt", type=str,
+                        default=BACKPORT_BRANCH,
+                        help="Backport branch format. "
+                             "Fields '{release}' and '{origbranch} will be "
+                             "replaced by the release name and remote branch "
+                             "name.")

default doc missing

> +                        help="Base the backport on this branch, "
+                             "default is the latest")
+    parser.add_argument("--backport-branch-fmt", type=str,
+                        default=BACKPORT_BRANCH,
+                        help="Backport branch format. "
+                             "Fields '{release}' and '{origbranch} will be "
+                             "replaced by the release name and remote branch "
+                             "name.")
+    parser.add_argument('-d', '--gitdir', type=str, default=os.getcwd(),
+                        help="Base git repo to work from")
+    parser.add_argument("PR", type=int, help="Pull request number to backport")
+    args = parser.parse_args()
+
+    gittoken = args.keyfile.read().strip()
+    g = GitHub(token=gittoken)
+    # TODO: exception handling

I guess this qualifies the script still to be WIP? ;-)

> +                        help="Backport branch format. "
+                             "Fields '{release}' and '{origbranch} will be "
+                             "replaced by the release name and remote branch "
+                             "name.")
+    parser.add_argument('-d', '--gitdir', type=str, default=os.getcwd(),
+                        help="Base git repo to work from")
+    parser.add_argument("PR", type=int, help="Pull request number to backport")
+    args = parser.parse_args()
+
+    gittoken = args.keyfile.read().strip()
+    g = GitHub(token=gittoken)
+    # TODO: exception handling
+    status, user = g.user.get()
+    if status != 200:
+        print("Could not retrieve user: {}".format(user['message']))
+        exit(1)

This

```py
status, obj = endpoint.get()
if status != 200:
   print("error")
   exit(val)
```

pattern shows up quite often. Any chance to put this into its own function?

> +        sys.exit(2)
+    if not pulldata['merged']:
+        print("Original PR not yet merged")
+        exit(0)
+    print("Fetching for commit: #{}: {}".format(args.PR, pulldata['title']))
+    orig_branch = pulldata['head']['ref']
+    status, commits = g.repos[ORG][REPO].pulls[args.PR].commits.get()
+    if status != 200:
+        print("No commits found for #{}: {}".format(args.PR,
+                                                    commits['message']))
+        sys.exit(3)
+    for commit in commits:
+        print("found {} : {}".format(commit['sha'],
+                                     commit['commit']['message']))
+
+    # Find latest release branch

General rule of thumb for me: If you can summarize a large chunk of code with a comment above it: Put it in its own function ;-).

> +    else:
+        status, branches = g.repos[ORG][REPO].branches.get()
+        if status != 200:
+            print("Could not retrieve branches for {}/{}: {}"
+                  .format(ORG,
+                          REPO,
+                          branches['message']))
+            sys.exit(4)
+        release_shortname, release_fullname = _get_latest_release(branches)
+        if not release_fullname:
+            print("No release branch found, exiting")
+            sys.exit(5)
+    print("Backport based on branch {}".format(release_fullname))
+
+    repo = git.Repo(args.gitdir)
+    # Fetch current upstream

Dito

> +        release_shortname, release_fullname = _get_latest_release(branches)
+        if not release_fullname:
+            print("No release branch found, exiting")
+            sys.exit(5)
+    print("Backport based on branch {}".format(release_fullname))
+
+    repo = git.Repo(args.gitdir)
+    # Fetch current upstream
+    upstream_remote = _get_upstream(repo)
+    if not upstream_remote:
+        print("No upstream remote found, can't fetch")
+        exit(6)
+    print("Fetching {} remote".format(upstream_remote))
+
+    upstream_remote.fetch()
+    # Build topic branch in temp dir

Dito

> +    bp_repo = git.Repo(worktree_dir)
+    # Apply commits
+    for commit in commits:
+        bp_repo.git.cherry_pick('-x', commit['sha'])
+    # Push to github
+    print("Pushing branch {} to origin".format(new_branch))
+    if not args.noop:
+        repo.git.push('origin', '{0}:{0}'.format(new_branch))
+    # Delete worktree
+    print("Pruning temporary workdir at {}".format(worktree_dir))
+    _delete_worktree(repo, worktree_dir)
+
+    labels = _get_labels(pulldata)
+    merger = pulldata['merged_by']['login']
+    if not args.noop:
+        # Open new PR on github

And so on...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/8968#pullrequestreview-196601402
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190125/a220fba2/attachment-0001.html>


More information about the notifications mailing list