Skip to content

Conversation

@arthurschreiber
Copy link
Contributor

This shows correct merge diffs, exactly in the way git will later
handle merges.

The previous method sometimes showed changes in the diff that were just
wrong.

(Basically, the diff shows now the output of git diff target_branch…source_branch).

This shows **correct** merge diffs, exactly in the way git will later
handle merges.

The previous method sometimes showed changes in the diff that were just
wrong.

(Basically, the diff shows now the output of `git diff
target_branch…source_branch`).
@dzaporozhets
Copy link
Contributor

hope you've checked it :)

@arthurschreiber
Copy link
Contributor Author

Heh. :D I was getting some really weird diffs before, and after that
change everything seems to be fine for new merge requests.

Anyway, if you've some more repos where you could test the output,
that would be great.

Am 21.04.2012 um 00:49 schrieb Dmitriy Zaporozhets
reply@reply.github.com:

hope you've checked it :)


Reply to this email directly or view it on GitHub:
#705 (comment)

@ariejan
Copy link
Contributor

ariejan commented Apr 23, 2012

You implicate that there is a bug in Gitlab. Please write a spec to support this and a fix to resolve that failing spec.

@arthurschreiber
Copy link
Contributor Author

Ha, I think I've to take that back. Neither the way that gitlab currently does diffs nor the way that I proposed in this pull request is right, actually. I'll open a new pull request with the correct code and an actual test case if I find some more free time.

@CedricGatay
Copy link
Contributor

@arthurschreiber yes I agree, I am actually trying to find out a correct way of doing diffs on merge requests. The current way of tracking diffs is not correct for me (serializing diffs to database is not the right way of doing it for me see #802). I hope I'll get enough time to work on this. You can hit me up if you want to discuss on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants