Add a --dry-run option #13
Conversation
- still fetches upstream changes - otherwise just prints commands that would be executed if everything runs without errors - also prints the PR creation URLs that would be opened - does NOT do any local checkouts, cherry-picks, branch creation or branch deletion - does NOT actually create any PRs Side effect: handles the cases where the remote to push to is called 'pr' and/or is checked out over SSH rather than HTTPS. Side effect: adds a "backport-" prefix to the cherry-pick branches
|
My git setup is somewhat different from yours (origin+pr as remotes, pr accessed over ssh rather than HTTPS), so I added a new dryrun option to figure out the details of the differences. There's one purely cosmetic change mixed in here, which was to add a "backport-" prefix to the branch names, as I found that easier to read in dryrun mode (vs the full hash on the cherry-pick lines), as well as making the branch names more self-explanatory. |
| @@ -5,16 +5,22 @@ | |||
|
|
|||
|
|
|||
| @click.command() | |||
| @click.option('--dryrun', is_flag=True) | |||
There was a problem hiding this comment.
Just minor nitpick, can we use --dry_run instead of --dryrun? :)
There was a problem hiding this comment.
Would you be OK with --dry-run? That will still give dry_run as the parameter name, and make the CLI spelling consistent with other tools like git and behave.
| return username | ||
|
|
||
|
|
||
| def run_cmd(cmd): | ||
| def run_cmd(cmd, *, dryrun=False): |
There was a problem hiding this comment.
nitpick, dry_run=False instead of dryrun=False
|
|
||
| In case of merge conflicts or errors, then... the script will fail :stuck_out_tongue: | ||
|
|
||
| Passing the `--dryrun` option will cause the script to print out all the |
There was a problem hiding this comment.
Let's add this to line 3
e.g.
python -m cherry_picker <commit_sha1> <branches> [--dry_run]
and maybe line 52?
| @@ -62,20 +69,27 @@ What this will do: | |||
|
|
|||
| (venv) $ git fetch upstream | |||
|
|
|||
| (venv) $ git checkout -b 6de2b78-3.5 upstream/3.5 | |||
| (venv) $ git checkout -b backport-6de2b78-3.5 upstream/3.5 | |||
There was a problem hiding this comment.
Please update the url in line 102
with
https://github.com/python/cpython/compare/3.5...Mariatta:backport-6de2b78-3.5?expand=1
| """ | ||
| cmd = "git remote get-url pr" | ||
| if run_cmd(cmd, dryrun=False): | ||
| return "pr" |
There was a problem hiding this comment.
How common is this "origin/pr" pattern?
I wanted the script to be as simple and straight-forward as possible, but perhaps the way to approach this is to pass it as a command line option.
eg. cherry_picker sha branches [--dry_run] [--push=pr]
if --push is not passed, then we'll use origin
There was a problem hiding this comment.
I actually learned origin/pr before upstream/origin, as the expected pattern was that you'd have a read-only clone of upstream first, and only decide to submit PRs later.
I like the "--push" CLI option though, leaving the automated guess solely for the upstream vs origin case.
|
Your suggested fixes all make sense to me, so there's just the |
|
--dry-run works :) Thanks. |
|
Please re-open the PR in core-workflow repo :) Thanks. |
everything runs without errors
creation or branch deletion
Side effect: handles the cases where the remote to push to is
called 'pr' and/or is checked out over SSH rather than HTTPS.
Side effect: adds a "backport-" prefix to the cherry-pick branches