-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Detect push target for local branches without upstream configuration #575
Comments
Hi! Ideally, the first step would be to use the existing Git logic for triangular workflows, see: These configurations are unfortunately under-used because under-advertised (and are missing command-line options) but are specifically geared toward triangular workflows (i.e. GitHub-style forking workflows). |
Adding a little more details: The way I configure my feature branches is that they track the upstream branch in which they will eventually be integrated, and I push them to my own fork. Basically the configs above enable a workflow like this: # first time only: setup 'origin' as the default push remote (I usually fork on GitHub, then clone my fork)
git config [--global] remote.pushDefault origin
# first time only: change default push behaviour to 'current'
git config [--global] push.default current
# create a feature branch
git fetch upstream
git checkout -b my-feature upstream/master
# work work work
# push my branch to my fork
git push # no other arguments needed ! (does `git push origin my-feature`)
# work work work
# rebase on latest master
git fetch # no other arguments needed! (does `git fetch upstream`)
git rebase # no other arguments needed! (does `git rebase upstream/master`)
# or in one step:
git pull --rebase # (does `git pull --rebase upstream master`)
# or if your project's workflow uses merge:
git fetch # no other arguments needed!
git merge # no other arguments needed! (does `git merge upstream/master`)
# or in one step:
git pull # (does `git pull upstream master`) So it enables a pretty cool workflow in my opinion, which feels really "natural". So it would be great if EDIT: here is a GitHub blog post dating from the release of Git 2.5, describing this workflow: https://github.blog/2015-07-29-git-2-5-including-multiple-worktrees-and-triangular-workflows/#improved-support-for-triangular-workflows |
This makes Is there a way to specify the PR URL when calling this? |
Is there a way to explicitly set the upstream config from github actions as a workaround for this? |
Maybe related (or not),
gh version 2.11.3 (2022-05-25) |
@stephanedaviet I would say it's related. To work around this, you can say:
|
Worth noting that git itself tells you not to use such a configuration (e.g. from
(not saying |
Good catch! It will make me re-examine my practice. |
@mislav I tried the same. Still couldn't create the pr. :'( |
EDIT: Spoke too soon, if you pull from any branch through magit it'll set the .merge config var and you'll run into the problem again. Quick breadcrumb for future generations: |
When using a triangular workflow with push.default = current and upstream being set to the target branch [1], or a similar but simpler central workflow [2], we should use @{push} instead to locate the remote branch. In fact, @{push} covers most cases in push.default = upstream too. The branch.<name>.merge is probably only needed when using RemoteURL and different remote / local branch names. [1] cli#575 (comment) [2] tpope/vim-fugitive#1172 (comment)
When using a triangular workflow with push.default = current and upstream being set to the target branch [1], or a similar but simpler central workflow [2], we should use @{push} instead to locate the remote branch. In fact, @{push} covers most cases in push.default = upstream too. The branch.<name>.merge is probably only needed when using RemoteURL and different remote / local branch names. [1] cli#575 (comment) [2] tpope/vim-fugitive#1172 (comment)
When using a triangular workflow with push.default = current and upstream being set to the target branch [1], or a similar but simpler central workflow [2], we should use @{push} instead to locate the remote branch. In fact, @{push} covers most cases in push.default = upstream too. The branch.<name>.merge is probably only needed when using RemoteURL and different remote / local branch names. [1] cli#575 (comment) [2] tpope/vim-fugitive#1172 (comment)
When using a push.default = current central workflow, we should use @{push} instead to locate the remote branch. In fact, @{push} covers most cases in push.default = upstream too. The branch.<name>.merge is probably only needed when using RemoteURL and different remote / local branch names. [1] cli#575 (comment) [2] tpope/vim-fugitive#1172 (comment)
Hey y'all! With the release of v2.66.1, we now support triangular workflows set up with remote.pushDefault, branch..pushRemote, and @{push} revision syntax for commands like We plan to add support to |
Expected Outcomes
Acceptance CriteriaGiven Given the Given the Given |
Two things that I use that I'm not sure are covered in the above criteria:
e.g. ❯ git remote -v
up https://github.mycorp.com/myorg/repo (fetch)
up https://github.mycorp.com/myorg/repo (push)
fork https://github.mycorp.com/me/repo.git (fetch)
fork https://github.mycorp.com/me/repo.git (push)
pub https://github.com/myorg/repo (fetch)
pub https://github.com/myorg/repo (push)
pubfork https://github.com/me/repo (fetch)
pubfork https://github.com/me/repo (push) The |
Good callout, @gibfahn. If not provided with the
Once these updates are in, I think that we should be able to add |
Commands such as
pr status
inspect the current branch and check its upstream configuration to see where it's pushed while resolving the branch to the associated PR.Some branches might be pushed but have no upstream configuration (simple
git push <remote> HEAD
withough-u
). Right now mechanisms such asprSelectorForCurrentBranch()
don't recognize such branches as being published to a remote. We should perhaps iterate over all remotes and assume one to be a push target if we find a same-named tracking branch on one. (This will perhaps be prone to false-positives, but it's a marked improvement over current behavior.)Reported by @rista404 #567 (comment)
The text was updated successfully, but these errors were encountered: