-
-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Fix 404 when click compare on non-forked repository #948
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
Conversation
This fix will reintroduce #304 |
OK. I will see that. |
10e2e97
to
1dd2c4c
Compare
@Bwko done. |
LGTM |
@@ -10,7 +10,7 @@ | |||
<div class="ui secondary menu"> | |||
{{if .PullRequestCtx.Allowed}} | |||
<div class="fitted item"> | |||
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch}}...{{.Username}}:{{.BranchName}}"> | |||
<a href="{{.BaseRepo.Link}}/compare/{{.BaseRepo.DefaultBranch}}...{{.BranchName}}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing {{.Username}}
fixes two-branches-inside-the-same-repo comparisons, but break comparisons across forks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about you just make sure that .Username
expands to what you expect it to expand, and later work on allowing different names ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the later work should be similar with github. Add two dropdown menus and list the forked repos.
LGTM |
I will close this one since it will break comparing across forks and I will send another one. |
Deprecated by #1104 |
Fix #936