Skip to content

Generate Diff and Patch direct from Pull head (#10936)#10938

Merged
lafriks merged 1 commit into
go-gitea:release/v1.11from
zeripath:backport-10936
Apr 3, 2020
Merged

Generate Diff and Patch direct from Pull head (#10936)#10938
lafriks merged 1 commit into
go-gitea:release/v1.11from
zeripath:backport-10936

Conversation

@zeripath
Copy link
Copy Markdown
Contributor

@zeripath zeripath commented Apr 3, 2020

Backport #10936

  • Generate Diff and Patch direct from Pull head

Fix #10932
Also fix "Empty Diff/Patch File when pull is merged"

Closes #10934

  • Add tests to ensure that diff does not change
  • Ensure diffs and pulls pages work if head branch is deleted too

Signed-off-by: Andrew Thornton art27@cantab.net

See below for a comment on why this code might not necessarily always be correct in 1.11

Backport go-gitea#10936

* Generate Diff and Patch direct from Pull head

Fix go-gitea#10932
Also fix "Empty Diff/Patch File when pull is merged"

Closes go-gitea#10934

* Add tests to ensure that diff does not change
* Ensure diffs and pulls pages work if head branch is deleted too

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.11.5 milestone Apr 3, 2020
@zeripath
Copy link
Copy Markdown
Contributor Author

zeripath commented Apr 3, 2020

This backport is potentially more questionable.

The same code is safe in 1.12 because we run a migration to update the mergebase correctly on PRs but 1.11 and earlier do not have this migration so old PRs will have the incorrect mergebase.

@lunny's suggest doctor command would help update the mergebase.

An alternative would be to have a mixture of the two solutions - one for if the head repo/head branch cannot be found or the PR is merged and the original code if not.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 3, 2020
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 3, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 3, 2020
@lafriks lafriks merged commit 99a364a into go-gitea:release/v1.11 Apr 3, 2020
@zeripath zeripath deleted the backport-10936 branch April 3, 2020 14:55
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants