-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
pull new/attach: Improve handling of the base branch #232
Comments
I think my ticket was referring to the scenario where the whole upstream repo doesn't exist, not just a branch. But I suspect this would solve it either way. |
Sounds good to me; feel free to close #174 in favor of this bug. There's one corner case this doesn't handle well: |
This actually sounds correct to me. If default branch was changed, most likely new PR based from old default one will have to be rebased on the new branch. At least that would be the case for our repos. |
I agree, for me it would not be all that unexpected to get PRs done against the default branch if there is no other configuration. The only other sane alternative I can think of is to remove step 1.iv. and just give an error if no branch was specified. Maybe that is a slightly more annoying option for the cases where what you want is to use the repo's default branch, but it at least guarantees no surprises, which is always a good thing. |
BTW, I agree with this too, but given there is a most likely means that in some cases we'll screw up if we assume this. So I think we should go with the safe option... |
Issue updated. |
There was some movement towards this. Now there is no hardcoded default branch to use as base (as of #174 - v2.1.0). We are still missing verification that a branch exists against GitHub. OTOH, people use different workflows, and I'm starting to think that using the tracking branch, or the default branch implicitly can always lead to surprises for certain use cases. Maybe to keep the usability boost of having useful defaults we can just add an option to disable guessing and always requiring All that said, creating a PR against a wrong branch is not all that bad as now the base branch can be easily changed... 🤔 |
We already have a few issues regarding on how the base branch is handled when creating pull requests.
One problem is following the tracking branch blindly. Another is to fallback to
hub.pullbase
(a somehow hardcoded value) blindly.The common problem seems to be not verifying whatever we do with GitHub first.
I think to come up with a holistic solution for this problem we should do the following to find out what the pull base should be:
Find a best pull base candidate:
If the(DONE)--base
argument was passed, used that, otherwise...hub.upstreamremote
, use that, otherwise... (pull: when creating a PR and the tracking branch is used, the repo should match hub.upstreamremote #296).If there is a(DONE)hub.pullbase
defined, use that, otherwise...default_branch
(Maybe? It might be too much guessing)Validate the pull base candidate by checking if the branch exists via the GitHub API.(Check the tracking branch points to the upstream repo when using it as the base branch for new pull requests #92)If it doesn't, quit with an error message asking the user use(DONE pull new: Error if no base branch can be inferred reliably #174)--base
explicitly.Validation should happen before the anything is pushed or the user is asked to edit a message.
@jwilk @mathias-baumann-sociomantic please confirm this covers the issues you created.
The text was updated successfully, but these errors were encountered: