-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Fix pull request compare link #1832
Conversation
.SignedUser maybe nil? |
Could you add some integration tests |
If I agree that it would be nice to not have to rely on dependencies between variables like this, but for the purpose of this PR I don't think a large-scale refactor is in order. |
It seems to me like an integration test for this would have to rely on finding and following the relevant link in the DOM. I'm a little nervous to add such a test, since it would be fragile if we ever change the underlying templates. If you have a better approach for an integration test, please let me know. |
I think it is ok to relay on DOM for integration tests, if UI is changed tests just need to be updated. |
LGTM |
LGTM |
The link should be
parentOwner/parent/compare/master...childOwner:master
instead ofparentOwner/parent/compare/master...parentOwner:master
.Without this fix, the owner of the child repo gets a 404 when they click on the compare-pull-request button on the parent repo's homepage.