Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a merge command to handle multi-parent commits in the version control system. The implementation allows users to merge multiple refspecs (branches/commits) and manages merge state persistence across operations.
- Introduces a new
mergecommand that can merge multiple refspecs into the current head - Adds merge state tracking with JSON serialization to handle interrupted merge operations
- Integrates merge functionality with the existing commit command to support multi-parent commits
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/merge.go | New file implementing the merge command with state persistence and refspec handling |
| cmd/commit.go | Modified to detect pending merges and pass additional parents to commit operations |
Comments suppressed due to low confidence (1)
Comment on lines
+77
to
+85
| if inProg { | ||
| err = MergeUnstowProgress(ctx, repoLoc, &mergeParams) | ||
| if err != nil { | ||
| logrus.Fatal("couldn't read the currently in-progress merge because: ", err) | ||
| } | ||
| } | ||
|
|
||
| mergeParams.ParentNames = append(mergeParams.ParentNames, heads...) | ||
| for _, head := range heads { |
There was a problem hiding this comment.
The ParentNames slice is being appended to even when a merge is already in progress. This will duplicate the current head and other refspecs on subsequent merge command invocations. The append should only happen when not in progress, or the slice should be reset first.
Suggested change
| if inProg { | |
| err = MergeUnstowProgress(ctx, repoLoc, &mergeParams) | |
| if err != nil { | |
| logrus.Fatal("couldn't read the currently in-progress merge because: ", err) | |
| } | |
| } | |
| mergeParams.ParentNames = append(mergeParams.ParentNames, heads...) | |
| for _, head := range heads { | |
| if inProg { | |
| err = MergeUnstowProgress(ctx, repoLoc, &mergeParams) | |
| if err != nil { | |
| logrus.Fatal("couldn't read the currently in-progress merge because: ", err) | |
| } | |
| mergeParams.ParentNames = nil // Reset ParentNames to prevent duplication | |
| } | |
| mergeParams.ParentNames = append(mergeParams.ParentNames, heads...) | |
| for _, head := range heads { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #35