-
-
Notifications
You must be signed in to change notification settings - Fork 60
Add --abort/--continue, --status, auto prefix commit msg with [X.Y] #68
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
Conversation
--abort: Do `git cherry-pick --abort` Clean up branch --continue Do `git commit -am "resolved" --allow-empty` Clean up branch Closes python#45
Backported this PR using the latest revision: python/cpython#1053 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a really nice refactoring & enhancement to me, just some minor questions/comments inline.
cherry_picker/cherry_picker.py
Outdated
def fetch_upstream(self): | ||
""" git fetch <upstream> """ | ||
# self.run_cmd(f"git fetch {self.upstream}") | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still meant to be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch! 😅 Re-enabled this.
cherry_picker/cherry_picker.py
Outdated
click.echo(f" dry-run: Create new PR: {url}") | ||
return | ||
webbrowser.open_new_tab(url) | ||
if '-' in cherry_pick_branch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpartition
should allow this function to be simplified a bit: https://docs.python.org/3/library/stdtypes.html#str.rpartition
Specifically, I think it's just:
prefix, sep, base_branch = cherry_pick_branch.rpartition('-')
return base_branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
cherry_picker/readme.rst
Outdated
:: | ||
|
||
-- dry-run Dry Run Mode. Prints out the commands, but not executed. | ||
-- push REMOTE Specify the branch to push into. Default is 'origin'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/branch/git remote/
cherry_picker/test.py
Outdated
|
||
def test_sorted_branch(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be handy to keep this test, either by splitting it out to a help function that's called by the property implementation, or else by testing cherry_picker.CherryPicker.sorted_branches.fget
directly on a dummy object with a suitable branches
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've been working on more tests. This is back in now. :)
Re-enable fetch from upstream more tests update readme
Lots of changes here:
--abort
and--continue
options(Described in (don't merge) Add --abort/--continue option to cherry_picker.py #65)
--status
option, will do git status in the CPython dir[X.Y]
#<PR-ID>
withGH-<PR-ID>
Closes #44
Closes #45