Skip to content

Fix --no-push related issues in --continue and --abort #43

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 3, 2022
Merged

Fix --no-push related issues in --continue and --abort #43

merged 6 commits into from
Oct 3, 2022

Conversation

Jackenmen
Copy link
Contributor

@Jackenmen Jackenmen commented Aug 7, 2021 •
edited
Loading

Fixes #41, fixes #42

@Jackenmen Jackenmen mentioned this pull request Aug 7, 2021
Closed
@Jackenmen Jackenmen changed the title Make the message made by cherry_picker --continue same as regular Fix --no-push related issues in --continue and --abort Aug 7, 2021
@Jackenmen Jackenmen force-pushed the issue/41 branch 2 times, most recently from 4d2133b to 784b11a Compare August 7, 2021 20:44
@Jackenmen
Copy link
Contributor Author

Right now this adds an already_committed to git config store to indicate whether the user is doing cherry_picker --continue after cherry_picker --no-push ... or after a cherry_picker ... that resulted in a conflict BUT I'm considering whether I won't change this to also support people doing manual git commit after resolving conflicts as mentioned here.

I'll check whether I could update the logic to just check whether there was already a commit made or not and post an update here once I do.

If one of the maintainers sees this before I get to it, does cherry-picker currently support commit ranges? This comment suggests that it does but this does not appear to be documented anywhere so I'm wondering whether I need to worry about supporting that too.

@Jackenmen
Copy link
Contributor Author

I'm considering whether I won't change this to also support people doing manual git commit after resolving conflicts as mentioned here.

I updated the logic to support this. Now, if there is exactly one commit in the backport branch, it has its commit message amended and otherwise, cherry_picker makes a new commit on its own.

@Jackenmen Jackenmen force-pushed the issue/41 branch 2 times, most recently from 1a32c1d to 4310297 Compare December 25, 2021 00:46
@Jackenmen Jackenmen marked this pull request as ready for review April 15, 2022 16:14
@Jackenmen
Fix --no-push related issues in --continue and --abort
f303fd2
@Jackenmen
Fix existing test
3d361f2
@Jackenmen
Add tests
8113c7e
@Jackenmen
Use different logic that should also work with manual use of git commit
a107630
@Jackenmen
Update tests
79a95a8
@Jackenmen
Remove test_abort_cherry_pick_fail
9873e90 
I'm not entirely convinced there is no way for `cherry-pick --abort`
to still fail but I can't think of a test case I could use to replace
this.
Due to newly added logic in `abort_cherry_pick()`, aborting is simply
skipped if there is no CHERRY_PICK_HEAD which is why in this case
it no longer fails.
@Jackenmen Jackenmen mentioned this pull request Aug 14, 2022
Closed
5 tasks
@Mariatta Mariatta merged commit b1647e8 into python:main Oct 3, 2022
@Jackenmen Jackenmen deleted the issue/41 branch October 4, 2022 06:42
Jackenmen added a commit to Jackenmen/cherry-picker that referenced this pull request Oct 4, 2022
@Jackenmen
Fix the test for backport pause and continue
3bc9c1d 
Introduced in python#43
@Jackenmen Jackenmen mentioned this pull request Oct 4, 2022
Merged
@Mariatta Mariatta added the hacktoberfest-accepted Accepted for hacktoberfest label Oct 4, 2022
Mariatta pushed a commit that referenced this pull request Oct 4, 2022
@Jackenmen
Fix the tests (#76)
061c63d 
* Fix the test for already existing branch

Introduced in #39

* Fix the test for backport pause and continue

Introduced in #43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mariatta Mariatta Mariatta approved these changes

Assignees
No one assigned
Labels
CLA signed hacktoberfest-accepted Accepted for hacktoberfest
Projects
None yet
Milestone
No milestone
3 participants
@Jackenmen @Mariatta @the-knights-who-say-ni