KEMBAR78
merge! by ethomson · Pull Request #1007 · libgit2/libgit2 · GitHub
Skip to content

Conversation

@ethomson
Copy link
Member

This merges a branch and puts the results in the workdir!

src/diff.c Outdated
Copy link
Member

Choose a reason for hiding this comment

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

By the looks of it, I think a single memcmp over the whole struct would be faster -- regardless of whether there are unset fields or not.

@vmg
Copy link
Member

vmg commented Oct 22, 2012

Gah, this is a lot of code. @arrbee, to the review-mobile.

@ethomson
Copy link
Member Author

@vmg Yeah, this is stupidly large. Let me break some of this stuff out into separate PRs.

@ethomson
Copy link
Member Author

So, here's what's still missing:

  • There's some hacky stuff that needs to get cleaned up (see TODOs)
  • - xxdiff and writing conflicting files does not respect EOL settings
  • - No detection for dirty index/workdir
  • - I want to spike dumping diff_tree and merging the results of two normal diffs. I thought a function like unpack_trees() would be useful, but I'm now less convinced
  • - No rename detection, I've done a few spikes that aren't integrated here yet
  • - The merge_opts / merge_tree_opts stuff is terrible. I'm thinking that there should be no high-level git_merge() command and callers should call git_merge_setup() / do the diff / deal with conflicts themselves. Need some feedback on this.
  • - Fastforward check is wrong (way too naive.)
  • - Can't merge two commits that don't have a common ancestor

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing you want to remove the (NOT IMPLEMENTED) comment? ⚡

@ethomson
Copy link
Member Author

ethomson commented Nov 5, 2013

Rebased!

@AnuiCuda
Copy link

AnuiCuda commented Nov 5, 2013

What's the status of this pull request? When can we expect it to hit the development branch?

@ethomson
Copy link
Member Author

ethomson commented Nov 5, 2013

@AnuiCuda Thanks for your interest. This is ready for code review and will need updates based on that review. Getting close!

@AnuiCuda
Copy link

AnuiCuda commented Nov 5, 2013

Sweet! Can't wait for this! I was about to go dig into the code to figure out what needed to be done when I tripped into this request. Nice work @ethomson!

@vmg
Copy link
Member

vmg commented Nov 5, 2013

@ethomson: Looks like we're having umask issues again...

@ethomson
Copy link
Member Author

ethomson commented Nov 5, 2013

Indeed. #1940 has the fix.

On Nov 5, 2013, at 5:58 AM, Vicent Martí notifications@github.com wrote:

@ethomson: Looks like we're having umask issues again...


Reply to this email directly or view it on GitHub.

@vmg
Copy link
Member

vmg commented Nov 5, 2013

Sounds good. Let's go for the rebase then. :)

@ethomson
Copy link
Member Author

ethomson commented Nov 5, 2013

@vmg Yup, rebased!

@vmg
Copy link
Member

vmg commented Nov 5, 2013

What could possibly go wrong.

vmg pushed a commit that referenced this pull request Nov 5, 2013
@vmg vmg merged commit 453e6b3 into libgit2:development Nov 5, 2013
phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014
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.

4 participants