Expand Merge API. #643
Expand Merge API. #643
Conversation
|
|
||
| // Verify that there is a staged entry. | ||
| Assert.Equal(1, repoStatus.Count()); | ||
| Assert.Equal(FileStatus.Staged, repo.Index.RetrieveStatus("a.txt")); |
There was a problem hiding this comment.
@ethomson - This test is currently failing because the entry is Modified. I think core git stages the uncommitted changes. Could this be a slight behavior difference in lg2?
There was a problem hiding this comment.
We should, too. Possible caching issue with the cache? Are we caching the cache? Hehe.
I was going to step through this, but it looks like the tests aren't quite cloning the merge repository correctly?
string path = CloneMergeTestRepo();
using (var repo = new Repository(path))
is failing for me because it can't open the path (it's not fully qualified for me). Travis is having the same issue, it looks like.
There was a problem hiding this comment.
Oh, it's not the fully qualifiedness. That repo you checked in had line ending changes applied!
There was a problem hiding this comment.
Aha, so I think this is a typo of a.txt where it should be looking for staged changes to b.txt. The normal_merge branch only changes b.txt from the common ancestor:
C:\Git\libgit2sharp\LibGit2Sharp.Tests\bin\Debug\TestRepos\b4df86fb\merge_testrepo_wd>git merge --no-commit normal_merge
Automatic merge went well; stopped before committing as requested
C:\Git\libgit2sharp\LibGit2Sharp.Tests\bin\Debug\TestRepos\b4df86fb\merge_testrepo_wd>git status
On branch master
All conflicts fixed but you are still merging.
(use "git commit" to conclude merge)
Changes to be committed:
modified: b.txt
|
@ethomson Thanks for your assistance with the wrong test logic and getting the test resources set up correctly! |
|
Wat? "Expand Merge API"? My dear @jamill, you should work on this 😉 This should be more like "Bless the world with PULL" or something extatic. 😍 |
| } | ||
| } | ||
|
|
||
| [Fact(Skip = "Skipping due to libgit2 issue")] |
There was a problem hiding this comment.
Failing due to libgit2/libgit2#2236
|
@nulltoken There is a lot of stuff in here, I think we should start winding this down. This includes features requested in #663, #662, #633. |
|
@jamill @nulltoken also #628 |
| public enum FastForwardStrategy | ||
| { | ||
| /// <summary> | ||
| /// Default fast-forward strategy. |
There was a problem hiding this comment.
Could we actually describe what the default is? (ie. Favor fast-forward when possible, otherwise create a merge commit)
|
@jamill Fantastic work 💖 |
This PR does not get you all the way there. I believe that libgit2 exposes the necessary knobs, we just need to hook them up. |
| @@ -336,6 +321,48 @@ static void DoFetch(RemoteSafeHandle remoteHandle, FetchOptions options, Signatu | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Pull changes from the configured upstream remote and branch into the current branch. | |||
There was a problem hiding this comment.
Could we also update those xml docs according to what has been done for Merge()
ie. into the current branch -> into the branch pointed at by HEAD.
|
@jamill Sorry for those additional comments ^^. Once you're done with them, could you please also update the native binaries with the latest version and squash? I'm eager to merge this! ❤️ |
|
libgit2 has been updated to latest and this this has been squashed into 2 commits. Thanks for the review! |
|
@jamill Could you please also update the |
merging branches. This includes a refactoring of the merge logic to support the new scenarios. New functionality includes: - Deprecate Network.Fetchheads, Repository.MergeHeads as these should be internal only. - Introduce ability to pull the configured upstream branch for the current branch - Introduce ability to merge a branch into the current branch. - Introduce options to control merge behavior. The current exposed options include whether to commit the merge commit and the allowed merge types.
|
@jamill Thank you so much for this 💎 💖 |
This includes several updates to the Merge API:
MergeOptionsparam, to specify the type of merge and whether to commit it or notFeedback on the API and / or testing approach would be welcome. I was trying to see if including a simple pre-populated merge repository would simplify the testing logic, and I think it does.
TODO: