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

Prevent server 500 on compare branches with no common history #6555

Merged
merged 3 commits into from
Apr 9, 2019

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Apr 9, 2019

This PR prevents the compare branch from sending a server 500 if there is no common history between branches. Instead it presents a simple compare page.

Fixes #6527 and probably others.

@zeripath
Copy link
Contributor Author

zeripath commented Apr 9, 2019

I think 1.8 could do with this too. It's far too easy to get a 500 with this endpoint.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 9, 2019
@techknowlogick
Copy link
Member

@zeripath I am not 100% sure about a backport of this due to new translation being required (usually translations aren't backported), although I do like that this prevents 500s. If anyone else has feedback on being pro/anti backport please add it to this PR.

@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 9, 2019
@codecov-io
Copy link

codecov-io commented Apr 9, 2019

Codecov Report

Merging #6555 into master will decrease coverage by <.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6555      +/-   ##
==========================================
- Coverage   40.36%   40.35%   -0.01%     
==========================================
  Files         405      405              
  Lines       54255    54260       +5     
==========================================
- Hits        21900    21899       -1     
- Misses      29340    29347       +7     
+ Partials     3015     3014       -1
Impacted Files Coverage Δ
modules/git/repo_pull.go 61.81% <25%> (-2.19%) ⬇️
modules/avatar/avatar.go 81.25% <0%> (-18.75%) ⬇️
modules/log/event.go 64.46% <0%> (-1.53%) ⬇️
modules/log/file.go 77.62% <0%> (+2.09%) ⬆️

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 7350e43...225cc32. Read the comment docs.

@lafriks
Copy link
Member

lafriks commented Apr 9, 2019

I'm for backport as it is better to get error in English than get 500 page

@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 9, 2019
@zeripath zeripath merged commit 89cc7c6 into go-gitea:master Apr 9, 2019
zeripath added a commit to zeripath/gitea that referenced this pull request Apr 9, 2019
…ea#6555)

* Prevent 500 if there is no common mergebase
* Prevent creation of PR with no history
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label Apr 9, 2019
zeripath added a commit to zeripath/git that referenced this pull request Apr 11, 2019
zeripath added a commit to go-gitea/git that referenced this pull request Apr 11, 2019
zeripath added a commit that referenced this pull request Apr 11, 2019
…#6558)

* Prevent server 500 on compare branches with no common history (#6555)
* Update code.gitea.io/git module
@zeripath zeripath deleted the fix-6527-no-500-if-no-history branch April 11, 2019 20:31
@zeripath
Copy link
Contributor Author

Backport done and merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created 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.

Gitea not able to handle un-mergable branches' PR
6 participants