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

Add Merge with ff-only #442

Closed
wants to merge 1 commit into from
Closed

Conversation

john-cai
Copy link
Contributor

@john-cai john-cai commented Jan 5, 2022

Add a Merge function that behaves like git merge. This is a first
iteration that only supports --ff-only, which is the simplest type of
merge.

Add a Merge function that behaves like git merge. This is a first
iteration that only supports --ff-only, which is the simplest type of
merge.
@sanderdekoning
Copy link

To others active in the repo could this be the start of implementing merge compatibility? (https://github.com/src-d/go-git/blob/master/COMPATIBILITY.md). Is this in development by anyone else? We can help (@john-cai)

@john-cai
Copy link
Contributor Author

@sanderdekoning yes, that's the goal. I wanted to start out with the simplest version and then iterate forward. I think next would be implementing the non-ff 3-way merge

@sanderdekoning
Copy link

That's great! I'll gladly try to assist and contribute. Do note that I'm new to this repository. Who would you say can assist where necessary/provide code review @john-cai? (@mcuadros?)

@john-cai
Copy link
Contributor Author

@sanderdekoning yes, a review sounds like a good start.

@DrewFromIQ
Copy link

Merge functionality would be very helpful for a project I'm working on at the moment. Would it be possible to get this PR completed? @mcuadros @smola

@rcontreras-te
Copy link

--ff-only merge would at least be a start. Just wondering why the PR hasn't even been considered, one way or the other, for... almost 11 months?

@john-cai
Copy link
Contributor Author

@mcuadros could you consider reviewing this PR?

@fhochleitner
Copy link

we would also very much appreciate a way of (at least) ff merging in the library. as we are implementing something for automatic merges during workflows and hate to escape from standard go implementation to command execution

@john-cai
Copy link
Contributor Author

john-cai commented May 4, 2023

@pjbgf would you mind reviewing and merging this one as it's been around for quite some time? thank you!

@pjbgf
Copy link
Member

pjbgf commented May 8, 2023

@john-cai I had a quick look on your PR and here's some initial feedback:

  • Although this PR only supports FF, it is important to make clear what the long-term API to support all merge options would look like, so that we ensure backwards compatibility as those features are implemented. MergeOptions.FFOnly as boolean seems quite restrictive, and does not seem to match the patterns we use across similar APIs (e.g. ResetMode).
  • Making IsFastForward public where it is does not feel like idiomatic Go code. I think it would probably be better placed as a method of EncodedObjectStorer.
  • The compatibility.md should be updated as part of this PR.

@alita1991
Copy link

Hi, is possible to also include support for merging with -Xours and -Xtheirs?

@patgmiller
Copy link

@john-cai are you still working on this? I might be interested in contributing to merge features and prefer not to duplicate efforts.

patgmiller added a commit to patgmiller/go-git that referenced this pull request Nov 4, 2023
@patgmiller patgmiller mentioned this pull request Nov 4, 2023
2 tasks
@asahasrabuddhe
Copy link

@patgmiller @john-cai I would like to see this happen too. I am happy to pledge my time to make this happen.

@oshiro3
Copy link

oshiro3 commented Feb 25, 2024

Many people, including myself, are looking forward to the merge feature being implemented in go-git.
But the merge function is huge and needs to be built on a series of small goals.
Perhaps the MergeOptions structure discussed in this PR should be the first thing to be decided rather than making it smaller.
Rather than continuing the discussion in this PR, I think we should open a discussions or some other place to determine the first goal for the implementation of the merge function.

@pjbgf
Copy link
Member

pjbgf commented Mar 9, 2024

Closing in favour of #1044.

@pjbgf pjbgf closed this Mar 9, 2024
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.

None yet

10 participants