Skip to content
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

git-imerge: update for the latest version #8118

Closed
wants to merge 4 commits into from

Conversation

abitrolly
Copy link
Contributor

  • The page is in the correct platform directory (common/, linux/, etc.)
  • The page has 8 or fewer examples.
  • The PR title conforms to the recommended templates.
  • The page follows the content guidelines.
  • The page description includes a link to documentation or a homepage (if applicable).

Version of the command being documented (if known): 1.2.0

@github-actions github-actions bot added the page edit Changes to an existing page(s). label Jun 7, 2022
abitrolly and others added 3 commits June 7, 2022 18:58
Co-authored-by: Axel Navarro <[email protected]>
Co-authored-by: Axel Navarro <[email protected]>
Co-authored-by: Axel Navarro <[email protected]>
@abitrolly
Copy link
Contributor Author

@navarroaxel thanks for review. These edits were made in hurry, and that's the consequences. (

Comment on lines +27 to +29
- Abort imerge operation and clean up temporary commits:

`git-imerge remove && git checkout {{previous_branch}}`
`git-imerge remove`
Copy link
Member

@waldyrious waldyrious Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I disagree with this change. The first part of the sentence ("Abort imerge operation") already refers to the cleaning up of the temporary commits. I don't think we need to specify that detail, especially since there's no mention of temporary commits before this.

In the same vein, I think it is better to keep the second part of the sentence and command, i.e. the extra step of returning to the original branch, as otherwise users would be left in a detached HEAD state. This is in fact recommended in the tool's documentation.

There are two issues for implementing a proper imerge-aborting subcommand (mhagger/git-imerge#123 and mhagger/git-imerge#189) but until then, this should not be presented stand-alone as if it were a fully supported operation of the git-imerge tool — it's more like plumbing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe replace that with git rebase --abort? If I remember correctly, it returns to the original branch automatically.

There are some things I need to do before I can check and confirm the behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think git rebase --abort would work; IIUC, the imerge operations aren't registered by git as a regular rebase. But do test and let us know :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I haven't used the merge capability, only rebase, and right now I don't have any PRs that need conflict resolution to test this. Looks like writing tests https://github.com/mhagger/git-imerge/tree/master/t would take a few hours, and I need them to complete other things.

Basically the testing plan.

  1. start rebase on a conflicting branch
  2. wait for terminal drop out for conflict resolution
  3. use remove and see if git switched back to conflicting
  4. check if temp branch is still there
  1. start merge on a conflicting branch
  2. wait for terminal drop out for conflict resolution
  3. use remove and see if git switched back to conflicting
  4. check if temp branch is still there
  1. start rebase on a conflicting branch
  2. wait for terminal drop out for conflict resolution
  3. use git rebase --abort and see if git switched back to conflicting
  4. check if temp branch is still there
  1. start merge on a conflicting branch
  2. wait for terminal drop out for conflict resolution
  3. use git merge --abort and see if git switched back to conflicting
  4. check if temp branch is still there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests for imerge would be awesome! Alternatively, in case writing the tests proves too burnensome, @mhagger himself could confirm whether git rebase --abort is a viable alternative to manually git checkouting (or git switching) to the previous branch.

@abitrolly abitrolly marked this pull request as draft June 8, 2022 10:41
@navarroaxel navarroaxel requested a review from waldyrious June 21, 2022 12:05
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

Hi all! This thread has not had any recent activity.
Are there any updates? Thanks!

@github-actions github-actions bot added the waiting Issues/PRs with Pending response by the author. label Jul 7, 2022
@marchersimon marchersimon removed the waiting Issues/PRs with Pending response by the author. label Jul 25, 2022
@github-actions
Copy link

Hi all! This thread has not had any recent activity.
Are there any updates? Thanks!

@github-actions github-actions bot added the waiting Issues/PRs with Pending response by the author. label Aug 10, 2022
@github-actions
Copy link

github-actions bot commented Sep 9, 2022

Hi everyone.
This thread is being closed as there was no response to the previous prompt.
However, please leave a comment whenever you're ready to resume, so the thread can be reopened.
Thanks again!

@github-actions github-actions bot closed this Sep 9, 2022
@abitrolly
Copy link
Contributor Author

I failed. Not sure I could address this even if being paid. It is just too complicated to resolve without spending a few days on answering all questions. I checked "Allow edits by maintainers", so feel free edit and commit it in any flavor. If it is not too late of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
page edit Changes to an existing page(s). waiting Issues/PRs with Pending response by the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants