Pull request revisions.

[ghost]

Fixes #12604
Fixes #5254

[zeripath]

Lint is failing due to a spelling mistake.

I before E except after C or sounding as Ay as in Weigh... and all that jazz...

[ghost]

2 things to note:

1. The owner/repo/compare/commit1...commit2 doesn't work correctly when the two commit has the same content but themselves are different. Example: create a PR with two commits, squash and then force push. Now you have two revisions. Comparing the second to the first one should show no additional change. It actually works perfectly on the Files changed pane in the PR itself. However if you do a full revision compare by clicking on compare to previous on the Revisions page (which will take you to the /compare/headhashv1...headhashv2 url) that will show all the files as changed. Its like a git show instead of a git diff. I looked at the code and I think maybe the compareinfo.mergehead is bad?
2. The revision creation is different for the creation of the PR and subsequent pushes. For subsequent pushes the revision is created in two steps. 1 is the git ref creation which is done at the post receive hook. And the database entry is created by the CreatePushPullComment later because only then is the pusher available.

[ghost]

Note to self: a separate patchset/difftree comparison page is needed. (to implement use: git patch, git diff-tree, git merge-base, git cherry)

Currently I believe that our compare page doesn't do the kind of shit that github does when comparing branches and listing how many commits behind ahead is the branch. This is good. A simple compare is basically should be a diff without the noise of that kind of stuff and a separate more detailed tree/patches compare page should exists.

[ghost]

Alright I will leave the diff tree feature to an other day. I will open an issue for it.

[codecov-commenter]

Codecov Report

Merging #12665 into master will increase coverage by 0.05%.
The diff coverage is 54.12%.

@@            Coverage Diff             @@
##           master   #12665      +/-   ##
==========================================
+ Coverage   43.07%   43.13%   +0.05%     
==========================================
  Files         658      661       +3     
  Lines       72451    72729     +278     
==========================================
+ Hits        31211    31373     +162     
- Misses      36183    36259      +76     
- Partials     5057     5097      +40     

Impacted Files	Coverage Δ
models/migrations/migrations.go	2.46% <ø> (ø)
models/migrations/v153.go	0.00% <0.00%> (ø)
modules/git/repo_commit.go	51.63% <ø> (+1.09%)	⬆️
routers/repo/pull.go	33.86% <38.59%> (+0.32%)	⬆️
modules/git/repo_ref.go	70.27% <50.00%> (-7.51%)	⬇️
models/revision.go	56.09% <56.09%> (ø)
routers/repo/middlewares.go	58.13% <57.14%> (-0.20%)	⬇️
modules/git/repo_revision.go	66.66% <66.66%> (ø)
services/pull/pull.go	41.64% <66.66%> (+0.86%)	⬆️
models/models.go	56.15% <100.00%> (+0.21%)	⬆️
... and 10 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dbca92...006103a. Read the comment docs.

[ghost]

Okay, I think it is good enough. I might create the more comprehensive tree comparsion page later.

[ghost]

[ghost]

The tree compare issue: #12800

[ghost]

I am not exactly sure why the tests with my fixtures are not working. @a1012112796 any idea how to fix the current unit test failures?

[ghost]

I added tests and fixed the issues.

@lunny @zeripath @a1012112796 Anything else?

[a1012112796]

Not suggest do it here.

[a1012112796]

Suggested change

	}
	return err
	}
	} else {
	return err
	}
	}

[ghost]

I think I have run out of time to make this work