Skip to content

Merge multiple heads #793

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

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Conversation

czarkoff
Copy link
Contributor

@czarkoff czarkoff commented Dec 8, 2024

This PR adds an ability to merge multiple heads. The operation can be started either from the commit view or from the branch list (second commit). As such merges often cannot be carried out automatically, the user is allowed to choose to use "ours" strategy for the merge, which allows to record a merge and lets user to continue (which would typically include cherry-picking needed commits manually and squashing them into the merge commit).

@love-linger
Copy link
Collaborator

I'm sorry. This is not the recommanded way to merge branches in SourceGit. In fact, supporting merging multiple branches doesn't simplify things, as it's more prone to conflicts.

Of the other software I have used, very few offer this functionality.

@czarkoff
Copy link
Contributor Author

czarkoff commented Dec 9, 2024

The point of this PR was not to simplify merges, but rather to allow performing multi-parent merges. Currently SourceGit user would have to perform those in terminal with a bit of additional effort of copy-pasting the branch names and/or commit IDs. That said, I understand that you might not be willing to provide and support UI for every git feature out there, so if this feature is not wanted, please go ahead and close this PR.

@love-linger
Copy link
Collaborator

I understand that some users like to use this way to merge multiple branches/commits at once. However, in this software, I do not recommend this. Because this action can be broken down into merging just one branch and doing it multiple times. This makes it easier to resolve conflicts or understand the entire commit history.

@love-linger
Copy link
Collaborator

love-linger commented Dec 9, 2024

Anthor reason that why I do not want to introduce this feature is that in most cases, the functions of different branches need to be merged into the main branch after the test of it has completed without issues, and merging multiple branches at once then test may make it difficult to findout which branch introduced the new issue.

@love-linger
Copy link
Collaborator

In my current working project, merging multiple branches at once is prohibited. Committers need to ensure the branch, which they want to merge into main branche, includes all necessary changes. And if necessary, they need to use commands such as squash/rebase to reorganize their commits before submitting the merge request. It is convenient for automated scripts to pull and test.

@czarkoff
Copy link
Contributor Author

czarkoff commented Dec 9, 2024

We used to have 2 cases when we did multiple parent merges:

  1. When we made "hotfix releases," that is, backported changes from the feature branches to the old stable branches. This practice was eventually abandoned for multiple reasons including the complexity of such merges.
  2. We maintain "deployment" branches - the branches where we merge the feature branches that are supposed to be deployed together to the testing servers. When some of the branches are then merged to the development head, we "rebase" the "deployment" branches by resetting the branch to the latest development HEAD and merging all non-merged feature branches. As our tree tends to have dozens of non-merged feature branches, merging these one-by-one produces a bit too much noise. Thus we attempt a multi-parent merge, and if that fails (more often then not), we do an "ours" merge with consequent cherry-pick of all commits from all those branches and squashing of all of that into a single merge commit.

@love-linger love-linger merged commit dce33fd into sourcegit-scm:develop Dec 9, 2024
13 checks passed
love-linger added a commit that referenced this pull request Dec 9, 2024
* do NOT modify the existing merge, and add a new constructor for `Commands.Merge` instead
* rewrite `ViewModels.MergeMultiple`
  - since `_histories.Commits.Find` may returns null, use `List<object>` instead of `List<Models.Commits>`
  - supports display merge target as both `Models.Commit` and `Models.Branch`
* rename translation key `Text.MergeMultiple.Commit` to `Text.MergeMultiple.Targets`, and add translations for zh_CN and zh_TW
* some UI/UX changes
@love-linger
Copy link
Collaborator

All right, you've convinced me.

I've merge this PR into develop branch, and add some changes to it. You can see commit 94daa46 for detail informations.

@czarkoff
Copy link
Contributor Author

czarkoff commented Dec 9, 2024

Thank you!

@czarkoff czarkoff deleted the merge_multiple branch December 14, 2024 19:02
NilsPvR added a commit to NilsPvR/sourcegit that referenced this pull request Dec 15, 2024
BranchCM.MergeMultiBranches, CommitCM.MergeMultiple, MergeMultiple sourcegit-scm#793
CommitCM.Merge 2053ce0
CommitDetail.Files.Search 894f3e9
Diff.UseBlockNavigation sourcegit-scm#703
FileCM.ResolveUsing 3b5d873
Hotkeys.Global.Clone bea2a39
InProgress.CherryPick.Head e1df5c5
InProgress.Merge.Operating ef40e4b
InProgress.Rebase.StoppedAt, Repository.Skip sourcegit-scm#790
InProgress.Revert.Head 93d9a04
Merge.Source 2504a52
WorkingCopy.CommitToEdit c136821
love-linger pushed a commit that referenced this pull request Dec 16, 2024
* localization: add missing de_DE keys

BranchCM.MergeMultiBranches, CommitCM.MergeMultiple, MergeMultiple #793
CommitCM.Merge 2053ce0
CommitDetail.Files.Search 894f3e9
Diff.UseBlockNavigation #703
FileCM.ResolveUsing 3b5d873
Hotkeys.Global.Clone bea2a39
InProgress.CherryPick.Head e1df5c5
InProgress.Merge.Operating ef40e4b
InProgress.Rebase.StoppedAt, Repository.Skip #790
InProgress.Revert.Head 93d9a04
Merge.Source 2504a52
WorkingCopy.CommitToEdit c136821

* localization: consistently use clone with 'k'

for most other keys a more "germanized" version with a k is used rather than a c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants