-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Ensure that cherry-picker doesn't exit with bad state on branch cleanup #61
Conversation
ff6ceb9
to
8e9a91f
Compare
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.
Reviewing the patch using the mobile app, I may be missing something but in general the idea seems sound. I don't see any immediate problems with the implementation but it's a good idea to have an extra set of eyes on this.
This whole git-worktree use case makes me think that further improvements of the cherry-picker could make use of temporary detached worktrees for the backporting process. Maybe even check out commits by hashes in detached HEAD mode in those worktrees so that the main dev env would remain unchanged.
There is also a problem that git worktrees share the |
This was missed in python#61
While I was fixing conflicts for #39, I noticed this has caused a small regression in the error message ( I fixed this in #39 (dc9bb66) since it caused tests to fail but I can split it if necessary. Note that #39 also adds a test that should ensure this doesn't happen in the future so it would save me some time from writing a dedicated regression test if we keep the fix in #39. |
* Fix exit with bad state when backport branch already exists * Add test * Fix branch name used in checkout error This was missed in #61 * Apply suggestions from code review Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com> Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
Fixes #17 and fixes #6
Does 2 things:
cleanup_branch()
method, I assume that this was already the intention anyway