Make hub compare work on forks
#2221
Conversation
|
Thank you for attempting to fix this, but this breaks CI checks. Can you look into that? |
sj26
force-pushed
the
implicit-compare-from-forks
branch
from
1ecd7ce
to
fcc5056
Compare
|
Sorry for the test oversight. I have rebased this on laster master and adjusted the tests. I also improved the implementation slightly. |
| """ | ||
| Usage: hub compare [-uc] [<USER>] [[<START>...]<END>] | ||
| hub compare [-uc] [-b <BASE>] | ||
| """ |
There was a problem hiding this comment.
These tests no longer made sense. If I'm on github.com and I fork a repository, then I jump in a terminal and clone my fork, then I push some commits on the default branch, then run git compare I should be sent to a comparison of the default branch on my fork, which might suggest I pull request those changes back to the forked repository. So opening a comparison from the default branch is now valid.
We might be able to do something more sensible if we knew the project was a fork or not, but I couldn't find a nice way to do this already in hub (project.IsFork()?), and without that information this seems the best decision.
|
|
||
| if flagCompareBase != "" { | ||
| if r == flagCompareBase { | ||
| utils.Check(command.UsageError("")) |
There was a problem hiding this comment.
This was to satisfy an existing test.
I use `git compare` without any arguments all the time when working on
feature branches in a one repository workflow - when pushing branches
directly to origin and using `git compare` to open a comparison for
review before opening a pull request. But it doesn't work for branches
pushed to a fork.
This change makes it understand the current branch might be pushed to
another remote and starts the comparison from there, then GitHub.com can
figure out the default base.
It still works great for single-repo workflows and still respects the
base flag for explicitly providing a base if required.
Before:
git clone hub
git fork
git checkout -b some-feature
...
git commit -m '...'
git push $USER HEAD -u
git compare
> Usage: hub compare [-uc] [<USER>] [[<START>...]<END>]
> hub compare [-uc] [-b <BASE>]
git compare -b origin:master
> Usage: hub compare [-uc] [<USER>] [[<START>...]<END>]
> hub compare [-uc] [-b <BASE>]
git remote -v
> origin https://github.com/github/hub.git (fetch)
> origin https://github.com/github/hub.git (push)
> sj26 https://github.com/sj26/hub.git (fetch)
> sj26 https://github.com/sj26/hub.git (push)
git compare github
> Usage: hub compare [-uc] [<USER>] [[<START>...]<END>]
> hub compare [-uc] [-b <BASE>]
/me gets frustrated
git compare github HEAD
# opens browser at:
# https://github.com/github/hub/compare/HEAD
git compare sj26 HEAD
# opens browser at:
# https://github.com/sj26/hub/compare/HEAD
/me tears out hair
After:
git clone hub
git fork
git checkout -b some-feature
...
git commit -m '...'
git push $USER HEAD -u
git compare
# opens browser at:
# https://github.com/sj26/hub/compare/some-feature
# which redirects to:
# github/hub@master...sj26:some-feature
/me happily reviews commits then opens a pull request
sj26
force-pushed
the
implicit-compare-from-forks
branch
from
fcc5056
to
cea50a5
Compare
| And I am on the "feature" branch with upstream "sj26/experimental" | ||
| When I successfully run `hub compare` | ||
| Then there should be no output | ||
| And "open https://github.com/sj26/hub/compare/experimental" should be run |
There was a problem hiding this comment.
I've also added a test for the new functionality.
|
This has gone super stale. Would you consider merging if I rebase it? |
I use
git comparewithout any arguments all the time when working on feature branches in a one repository workflow - when pushing branches directly to origin and usinggit compareto open a comparison for review before opening a pull request. But it doesn't work for branches pushed to a fork.This change makes it understand the current branch might be pushed to another remote and starts the comparison from there, then GitHub.com can figure out the default base.
It still works great for single-repo workflows and still respects the base flag for explicitly providing a base if required.
Before:
After:
(I used this branch to open this PR!)