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

Write merge conflicted files when pulling #559

Merged
merged 8 commits into from
Mar 3, 2016

Conversation

pietbrauer
Copy link
Member

This mimics the default git behaviour and writes the conflicted files down like this:

<<<<<HEAD
fksdjfa;lsdkjf;lads;afl
========
<<<<<<<<<4567dvssqw3124323

It also writes into .git/MERGE_HEAD the remote commit SHA, into .git/MERGE_MSG the correct merge message. It can then be pulled in the merge commit via try repository.preparedMessage().

The merge head enumeration was added so the merge commit can pull all merge heads and use them as parent commits.

@pietbrauer
Copy link
Member Author

This has tests now ✅

@pietbrauer
Copy link
Member Author

I revised this PR and move the merge algorithm to it's own function so it can be used independently when merging branches.

The reason I did this was because I was beginning to implement merging branches in Git2Go and saw that is was exactly the same procedure as pulling and we should probably factor it out. This also made pulling way easier to grasp.

I am open to suggestions regarding the naming of the merge function.


/// Merge Branch into current branch
///
/// analysis - The resulting analysis.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this isn't an argument anymore.

@joshaber joshaber self-assigned this Mar 3, 2016
/// fromBranch - The branch to merge from.
/// error - The error if one occurred. Can be NULL.
///
/// Returns YES if the nerge was successful, NO otherwise (and `error`, if provided,
Copy link
Member

Choose a reason for hiding this comment

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

s/nerge/merge 🍷

GTRepository *repository = entriesPayload->repository;
GTRepositoryEnumerateMergeHeadEntryBlock enumerationBlock = entriesPayload->enumerationBlock;

GTCommit *commit = [repository lookUpObjectByOID:[GTOID oidWithGitOid:oid] objectType:GTObjectTypeCommit error:NULL];
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if the merge head methods should yield OIDs instead of commits, so users don't incur the lookup overhead unless they actually need the commits. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, you basically take the results of this method and put it into the parents of the merge commit. I mean it's only a matter of another map so I omitted that. But we totally could switch to GTOIDs as this would map 1:1 to the libgit2 functionality.

@joshaber
Copy link
Member

joshaber commented Mar 3, 2016

💃 Just the one question left.

@pietbrauer
Copy link
Member Author

@joshaber Changed the implementation to return GTOID objects 🚀

@pietbrauer
Copy link
Member Author

Would be nice to have a new version including this and Bitcode ✌️

@joshaber
Copy link
Member

joshaber commented Mar 3, 2016

🤘

joshaber added a commit that referenced this pull request Mar 3, 2016
@joshaber joshaber merged commit b52842f into master Mar 3, 2016
@joshaber joshaber deleted the piet/write-merge-conflicted-files branch March 3, 2016 18:29
@pietbrauer pietbrauer mentioned this pull request Mar 8, 2016
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