Find PRs using `@{push}` #9208

Frederick888 · 2024-06-14T05:39:54Z

`f9a1329f` `Find PRs using @{push}`

When using a push.default = current central workflow [1], 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] https://github.com/tpope/vim-fugitive/issues/1172#issuecomment-522301607

`a049eade` `Only find PRs w/ branch.<name>.merge if push.default = upstream/tracking`

When push.default is not 'upstream' / 'tracking' (or 'nothing'), we can
expect local and remote branch names to be the same and solely rely on
@{push} or RemoteURL.

This fixes the wrong error message 'no pull requests found for branch
"<target branch>"' when the local branch is not pushed in the
push.default = simple / current and upstream = <target branch> setup.

`285a5df0` `Find push remote using branch.<name>.pushRemote and remote.pushDefault`

When using a push.default = current triangular workflow, apart from
using @{push} to determine the remote branch name, we should also follow
the

1. branch.<name>.pushRemote
2. remote.pushDefault
3. branch.<name>.remote

...list to determine which remote Git pushes to.

This fixes things like gh pr view in a push.default = simple / current + upstream pointing to target branch.

It does not fix the issues with gh pr create.

Related to: #575.

williammartin · 2024-07-10T10:22:30Z

Wow this is an oooooold issue. Thank you for your contribution. The details of the original issue look quite involved, so it may take me some time to fully grok and review this. Cheers.

williammartin · 2024-07-10T18:12:27Z

Hey @Frederick888, can you provide some more context about the end-to-end workflow here? I believe this relates to triangular workflows where there are 2 remotes, one configured for push and one for fetch but I think I'm missing something in my understanding.

Here's what I tried locally:

Clone my fork and check the remotes:

➜ gh repo clone williammartin/cli
Cloning into 'cli'...
...
! Repository cli/cli set as the default repository. To learn more about the default repository, run: gh repo set-default --help

➜ cd cli && git remote -v
origin  https://github.com/williammartin/cli.git (fetch)
origin  https://github.com/williammartin/cli.git (push)
upstream        https://github.com/cli/cli.git (fetch)
upstream        https://github.com/cli/cli.git (push)

Configure the triangular workflow:

➜ git config remote.pushDefault origin
➜ git config push.default current

Create a PR from my fork using the syntax mentioned by Mislav:

➜ git checkout -b wm/9208-triage && git commit --allow-empty -m "9208-triage" && git push
Switched to a new branch 'wm/9208-triage'
[wm/9208-triage 6b499b8e] 9208-triage
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), 189 bytes | 189.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0), pack-reused 0
remote:
remote: Create a pull request for 'wm/9208-triage' on GitHub by visiting:
remote:      https://github.com/williammartin/cli/pull/new/wm/9208-triage
remote:
To https://github.com/williammartin/cli.git
 * [new branch]        wm/9208-triage -> wm/9208-triage

➜ gh pr create -H williammartin:wm/9208-triage

Creating pull request for williammartin:wm/9208-triage into trunk in cli/cli

? Title 9208-triage
? Choose a template PULL_REQUEST_TEMPLATE.md
? Body <Received>
? What's next? Submit
https://github.com/cli/cli/pull/9306

Then I expect using gh pr view to not find the PR (the original issue):

➜ gh pr view
no pull requests found for branch "wm/9208-triage"

But I anticipated that this PR would fix it:

➜ ~/workspace/cli/bin/gh pr view
no pull requests found for branch "wm/9208-triage"

So what am I misunderstanding? Thanks!

williammartin · 2024-07-10T18:19:15Z

Ah, perhaps what I'm missing is that when we create the local branch it needs to be checked out from the upstream?

Deleting the old branch:

➜ git branch -D wm/9208-triage

Creating a new branch from upstream/trunk:

➜ git checkout -b wm/9208-triage upstream/trunk
branch 'wm/9208-triage' set up to track 'upstream/trunk'.
Switched to a new branch 'wm/9208-triage'

Showing the difference between current release and this PR:

➜ gh pr view
no pull requests found for branch "trunk"

➜  ~/workspace/cli/bin/gh pr view | cat
title:  9208-triage
state:  OPEN
author: williammartin
labels:
assignees:      williammartin
reviewers:
projects:       The GitHub CLI (Needs review 🤔)
milestone:
number: 9306
url:    https://github.com/cli/cli/pull/9306
additions:      0
deletions:      0
auto-merge:     disabled
--
# Description

Triaging #9208, you can safely ignore this.

Is this expected behaviour? I'm trying to figure out why it would be important to gh pr view that the branch is created from upstream.

williammartin · 2024-07-10T18:53:47Z

I think the issue here is that when the branch starting point doesn't contain a remote, then the config doesn't contain enough information about the upstream repo, which is where the PR actually lives:

➜ git config --get-regexp "^branch.wm/9208-triage.(remote|merge|pushremote)$" | cat
branch.wm/9208-triage.remote upstream
branch.wm/9208-triage.merge refs/heads/trunk

➜ git checkout -b wm/9208-triage-no-upstream
Switched to a new branch 'wm/9208-triage-no-upstream'

➜ git config --get-regexp "^branch.wm/9208-triage-no-upstream.(remote|merge|pushremote)$" | cat
<EMPTY>

Which is an issue for https://github.com/cli/cli/pull/9208/files#diff-ccedca01e076cd065987253b366873ca2bee2e28c9d0bc13d50d7b7795c0bf4eR328

I guess the solution to this would be as Mislav suggested:

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.

But that seems out of scope for this, if I understand your goal correctly.

Do I understand this correctly @Frederick888?

Frederick888 · 2024-07-11T11:36:28Z

can you provide some more context about the end-to-end workflow here? I believe this relates to triangular workflows where there are 2 remotes, one configured for push and one for fetch but I think I'm missing something in my understanding.

It's actually related to both central workflow and triangular.

First of all, make sure you've done git config --global push.default current. Then:

Central:

$ git clone https://github.com/Frederick888/playground.git
Cloning into 'playground'...
remote: Enumerating objects: 164, done.
remote: Counting objects: 100% (68/68), done.
remote: Compressing objects: 100% (39/39), done.
remote: Total 164 (delta 40), reused 55 (delta 29), pack-reused 96
Receiving objects: 100% (164/164), 39.82 KiB | 1.99 MiB/s, done.
Resolving deltas: 100% (56/56), done.
$ cd playground
$ git checkout --guess test-diff
branch 'test-diff' set up to track 'origin/test-diff'.
Switched to a new branch 'test-diff'
$ git branch --set-upstream-to origin/master
branch 'test-diff' set up to track 'origin/master'.
$ cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "origin"]
        url = https://github.com/Frederick888/playground.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
        remote = origin
        merge = refs/heads/master
[branch "test-diff"]
        remote = origin
        merge = refs/heads/master

# with my patch
$ gh pr view
test diff Frederick888/playground#7
Draft • Frederick888 wants to merge 4 commits into master from test-diff • less than a minute ago
+8 -1 • - Checks pending
[ ... ]
View this pull request on GitHub: https://github.com/Frederick888/playground/pull/7

# trunk 0df5596512ad44dec74ab3dc07a4e6ea3a7c78fc
$ ~/Programming/Go/gh-cli/bin/gh pr view
no pull requests found for branch "master"

Triangular:

$ git clone https://github.com/cli/cli.git
Cloning into 'cli'...
remote: Enumerating objects: 54449, done.
remote: Counting objects: 100% (1458/1458), done.
remote: Compressing objects: 100% (817/817), done.
remote: Total 54449 (delta 811), reused 1155 (delta 629), pack-reused 52991
Receiving objects: 100% (54449/54449), 29.40 MiB | 6.75 MiB/s, done.
Resolving deltas: 100% (36967/36967), done.
$ cd cli
$ git remote rename origin upstream
Renaming remote references: 100% (71/71), done.
$ gh repo fork --remote --remote-name origin
! Frederick888/cli already exists
✓ Added remote origin
$ git config branch.trunk.pushremote upstream
$ git config remote.pushdefault origin
$ gh repo set-default cli/cli
✓ Set cli/cli as the default repository for the current directory
$ git fetch --all
$ git checkout --guess find-pr-by-rev-parse-push
branch 'find-pr-by-rev-parse-push' set up to track 'origin/find-pr-by-rev-parse-push'.
Switched to a new branch 'find-pr-by-rev-parse-push'
$ git branch --set-upstream-to upstream/trunk
branch 'find-pr-by-rev-parse-push' set up to track 'upstream/trunk'.
$ cat .git/config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "upstream"]
        url = https://github.com/cli/cli.git
        fetch = +refs/heads/*:refs/remotes/upstream/*
        gh-resolved = base
[branch "trunk"]
        remote = upstream
        merge = refs/heads/trunk
        pushremote = upstream
[remote "origin"]
        url = [email protected]:Frederick888/cli.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[remote]
        pushdefault = origin
[branch "find-pr-by-rev-parse-push"]
        remote = upstream
        merge = refs/heads/trunk

# with my patch
$ gh pr view | cat
title:  Find PRs using `@{push}`
state:  OPEN
author: Frederick888
labels: external
assignees:
reviewers:       (Requested), williammartin (Requested)
projects:       The GitHub CLI (Needs review 🤔)
milestone:
number: 9208
url:    https://github.com/cli/cli/pull/9208
additions:      246
deletions:      35
auto-merge:     disabled

# trunk 0df5596512ad44dec74ab3dc07a4e6ea3a7c78fc
$ ~/Programming/Go/gh-cli/bin/gh pr view | cat
no pull requests found for branch "trunk"

When you check out a new branch, there are some similar things you need to do. But the key is, after pushing the new branch, making sure git rev-parse --abbrev-ref <branch>@{push} prints out the correct remote and branch for both target and feature branches.

I've got a super-charged ~~atrocious~~ git co alias to help me do most these.

I guess the solution to this would be as Mislav suggested:

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.

#575 is a bit more than this issue. What Mislav originally mentioned sounds like something gh could do when a user essentially doesn't have a correct setup and always uses the full-formed git push origin feat:feat or something + even git config branch.<branch>.remote = ..

What I'm trying to address here however, is a perfectly valid setup. All the info that we need to determine the target / feature branches are already in gitconfig. Git itself understands the setup already. It's only that gh's not consuming the info right. While I'm not sure what the best approach should be in the scenario Mislav described, there is no need to deploy such last-resort method here in my setup.

But that seems out of scope for this, if I understand your goal correctly.

Do I understand this correctly @Frederick888?

So yeah, you were on the right track 👍

williammartin · 2024-07-24T14:58:03Z

@Frederick888, I started extracting issues to describe each of these enhancements in detail e.g. #9363

I'm having trouble understanding the second commit in this series.

It seems like the first commit makes it so that:

If @{push} resolves to something correctly, then we use that
Otherwise we use the merge config entry

And after the second commit it makes it so that:

If @{push} resolves to something correctly, then we use that
Only if push.default is empty or upstream then we use the merge config entry
Otherwise ??

I don't understand what is expected to happen in this last case. I don't understand what this change is intended to protect. Thanks!

Frederick888 · 2024-07-25T02:04:20Z

@williammartin

* Only if `push.default` is empty or `upstream` then we use the `merge` config entry

Actually here it uses MergeRef only if push.default is upstream, not when it's empty (since simple is the default push.default).

* Otherwise ??
I don't understand what is expected to happen in this last case. I don't understand what this change is intended to protect. Thanks!

Otherwise it uses current branch AFAIU

cli/pkg/cmd/factory/default.go

Lines 150 to 158 in 0df5596

    
           func branchFunc(f *cmdutil.Factory) func() (string, error) { 
        
           	return func() (string, error) { 
        
           		currentBranch, err := f.GitClient.CurrentBranch(context.Background()) 
        
           		if err != nil { 
        
           			return "", fmt.Errorf("could not determine current branch: %w", err) 
        
           		} 
        
           		return currentBranch, nil 
        
           	} 
        
           }

Since @{push} can only be rev-parsed if a user has already pushed, even if a user is using push.default = current (or simple by default which expects 'you [...] to configure an upstream branch with the same name'), before they actually push, without the 2nd commit, branchConfig.Push is empty and gh will pick up branchConfig.MergeRef and report 'no pull requests found for branch "master/main/trunk"', while it should've reported 'no pull requests found for branch "<current feat branch>"'.

williammartin · 2024-07-25T06:58:16Z

Ah right, I see. Because simple is the default, we can just assume that the name of the remote branch is the same as the local branch.

I'm trying to imagine whether this could have broken any other push.default configurations:

nothing
tracking (which is deprecated but does mean the same as upstream)
matching (this seems absolutely ridiculous 😅)

I suppose matching might be broken but it seems mad. Not sure whether we care to support deprecated tracking, though I suppose it's an easy change. nothing seems like it would already not work.

Do you agree?

Frederick888 · 2024-07-25T07:18:32Z

I'm trying to imagine whether this could have broken any other push.default configurations:
* `nothing`

Such users are probably of those who always use git push origin local-branch:remote-branch (via some GUIs presumably), which is actually more relevant to the original #575. I'd say my PR neither makes this situation better nor worse.

* `tracking` (which is deprecated **but** does mean the same as `upstream`)

Oh yup, good spot! I absolutely should've added it too. b7521e1 and I'll reword the earlier commit too.

* `matching` (this seems absolutely ridiculous 😅)

Yes I doubt many users actually use this locally. But still it expects 'all branches having the same name on both ends'. So it conveniently falls into the 'otherwise' category.

Frederick888 · 2024-07-25T07:43:31Z

@williammartin Done 😄 (Gonna have to head off soon but let me know if you have other concerns.)

williammartin · 2024-07-26T15:13:50Z

Under what circumstances could there be a remote branch that has a PR open from it, but @{push} doesn't resolve when push.default = current?

The only case I can imagine is that we checkout a new local branch with the same name as a remote branch but that isn't tracking it. Is that a likely scenario?

williammartin · 2024-07-26T16:36:07Z

Apart from my question above I believe I've captured the value of these changes in the following issues:

pr view and pr status should respect @{push} where possible #9363
pr view and pr status should respect with remote.pushdefault #9364
pr view and pr status should respect branch.<name>.pushRemote #9365
pr view and pr status should work with push.default = simple even if the branch is set to merge from a different remote branch #9374

Frederick888 · 2024-07-30T11:25:51Z

Under what circumstances could there be a remote branch that has a PR open from it, but @{push} doesn't resolve when push.default = current?

The only case I can imagine is that we checkout a new local branch with the same name as a remote branch but that isn't tracking it. Is that a likely scenario?

Actually since push.default = current assumes local and remote branch names are always the same, as long as there is a branch with the same name in the push remote (as per the list in the 3rd commit), the local branch is considered to be tracking it.

For example, I can even just branch off of a random commit with the name find-pr-by-rev-parse-push, and then @{push} will point to this PR:

$ git checkout trunk
$ git branch -D find-pr-by-rev-parse-push
$ git show
commit b05bddc210a1e45a25b76f9b320239e4c36f03ee (HEAD -> trunk, upstream/trunk, upstream/HEAD)
Merge: 0d87ddaca 58e619677
Author: Andy Feller <[email protected]>
Date:   Mon Jul 29 17:31:20 2024 -0400

    Merge pull request #9344 from babakks/9292-add-remove-milestone-option

    Add `--remove-milestone` option to `issue edit` and `pr edit`

$ git checkout -b find-pr-by-rev-parse-push
Switched to a new branch 'find-pr-by-rev-parse-push'
$ git rev-parse --abbrev-ref @{push}
origin/find-pr-by-rev-parse-push

So the only case where that can happen is when the user didn't fetch the remote branch, e.g.

# continue above
$ git branch -d -r origin/find-pr-by-rev-parse-push
Deleted remote-tracking branch origin/find-pr-by-rev-parse-push (was ad62216b0).
$ git rev-parse --abbrev-ref @{push}
@{push}
fatal: ambiguous argument '@{push}': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

williammartin · 2024-07-30T12:24:06Z

Yeh right, so when push.default = current the second commit is only relevant if the remote branch hasn't been fetched. I'm just ensuring I understand the scenarios at play here. That seems like an unlikely scenario. I understand why it is relevant for push.default = simple, wherein @{push} won't resolve when merge points to a different branch, and yet we know the remote must have the same name.

Frederick888 · 2024-07-30T12:37:30Z

Yeh right, so when push.default = current the second commit is only relevant if the remote branch hasn't been fetched.

Or the user hasn't pushed.

That seems like an unlikely scenario.

I agree. Even if a user is dealing with a very active large repo and always only fetches a few branches selectively, it doesn't make too much sense to skip the current remote tracking branch.

I understand why it is relevant for push.default = simple, wherein @{push} won't resolve when merge points to a different branch, and yet we know the remote must have the same name.

Yup and just tested that git push would exit with an error too, so push.default = simple + different local & remote branch names is really just an invalid setup. It should be safe to let @{push} and the 'otherwise' take care of it.

$ g config push.default simple
$ g checkout --track origin/find-pr-by-rev-parse-push
branch 'find-pr-by-rev-parse-push' set up to track 'origin/find-pr-by-rev-parse-push'.
Switched to a new branch 'find-pr-by-rev-parse-push'
$ g config branch.find-pr-by-rev-parse-push.merge refs/heads/trunk
$ g rev-parse --abbrev-ref @{push}
fatal: cannot resolve 'simple' push to a single destination
$ g push
fatal: The upstream branch of your current branch does not match
the name of your current branch.  To push to the upstream branch
on the remote, use

    git push origin HEAD:trunk

To push to the branch of the same name on the remote, use

    git push origin HEAD

To avoid automatically configuring an upstream branch when its name
won't match the local branch, see option 'simple' of branch.autoSetupMerge
in 'git help config'.

williammartin · 2024-07-30T13:25:17Z

Or the user hasn't pushed.

If the user hasn't pushed then there can't be a PR open to view or status, right?

This is what I was describing above. The only scenario I can imagine there being a PR to view, a user having push.default = current and @{push} not resolving correctly is if the user hasn't fetched the remote branches so they just happen to have a branch of the same name as a remote one.

Frederick888 · 2024-07-30T13:30:24Z

Or the user hasn't pushed.

If the user hasn't pushed then there can't be a PR open to view or status, right?

This is what I was describing above. The only scenario I can imagine there being a PR to view, a user having push.default = current and @{push} not resolving correctly is if the user hasn't fetched the remote branches so they just happen to have a branch of the same name as a remote one.

Yup that's right. Thanks for the clarification.

I was talking about that gh needed to report the correct name of the missing remote tracking branch if the user hadn't pushed :)

williammartin · 2024-07-30T13:36:27Z

Got it! Nice one.

Ok, just for your information I am now on vacation, however, I just wanted to finish all the open parts of this discussion. I'd hoped to get this merged before leaving but couldn't due to some unexpected personal circumstances. You shouldn't expect to hear back from me until August 15th or 19th, as I am taking a much needed long break. I'm sorry for the delay. Hopefully I won't have to ask you to remember too much on my return 😅

Thanks for your continued involvement and clarifications, it's been extremely valuable in giving me confidence in getting this change in when I return.

Frederick888 · 2024-07-30T13:39:15Z

Ok, just for your information I am now on vacation, however, I just wanted to finish all the open parts of this discussion. I'd hoped to get this merged before leaving but couldn't due to some unexpected personal circumstances. You shouldn't expect to hear back from me until August 15th or 19th, as I am taking a much needed long break. I'm sorry for the delay. Hopefully I won't have to ask you to remember too much on my return 😅

No worries at all! Enjoy your break!

…

On 30/7/24 23:36, William Martin wrote: Got it! Nice one. Ok, just for your information I am now on vacation, however, I just wanted to finish all the open parts of this discussion. I'd hoped to get this merged before leaving but couldn't due to some unexpected personal circumstances. You shouldn't expect to hear back from me until August 15th or 19th, as I am taking a much needed long break. I'm sorry for the delay. Hopefully I won't have to ask you to remember too much on my return 😅 Thanks for your continued involvement and clarifications, it's been extremely valuable in giving me confidence in getting this change in when I return. — Reply to this email directly, view it on GitHub <#9208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABCMP7Z3SDWBSBC3W35CX5DZO6JHDAVCNFSM6AAAAABJJVOOYWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJYGM3TEMJTG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Frederick Zhang PGP: 8BFB EA5B 4C44 BFAC C8EC 5F93 1F92 8BE6 0D8B C11D

benknoble · 2024-11-12T14:40:05Z

I've just skimmed the conversation, but it looks like all the major questions have been resolved and the fixes documented in linked issues.

Are there any remaining barriers to merging this? It would be a huge quality of life improvement for me. Right now I either have to do

gh pr view -w <type the branch name>

(autocomplete doesn't work) or

gh pr view -w $(git rev-parse --symbolic-full-name @ | cut -d/ -f 3-)

for the current branch, since my @{push} and current branch share a name.

It seems like maybe @williammartin just hasn't checked in since his return 😅 Thanks for all the effort you both put in to creating and documenting this!

williammartin · 2024-11-12T14:44:15Z

It seems like maybe @williammartin just hasn't checked in since his return

😅 I check it every week, and then I am sad that I've left @Frederick888 hanging for so long.

Are there any remaining barriers to merging this?

Purely a matter of prioritising the review. We had a lot of work in the lead up to GitHub universe and some unexpected work after. We're intending to spend a week on getting through these outstanding PRs, I'm hopeful it will happen in the next week or two but I can't commit to that.

Did you actually try this branch from source by any change @benknoble ?

benknoble · 2024-11-12T14:46:29Z

It seems like maybe @williammartin just hasn't checked in since his return

😅 I check it every week, and then I am sad that I've left @Frederick888 hanging for so long.

Ah, my apologies! And a second thank you for your diligence.

Are there any remaining barriers to merging this?

Purely a matter of prioritising the review. We had a lot of work in the lead up to GitHub universe and some unexpected work after. We're intending to spend a week on getting through these outstanding PRs, I'm hopeful it will happen in the next week or two but I can't commit to that.

Makes sense, thank you for your transparency.

Did you actually try this branch from source by any change @benknoble ?

No, but I will today. Good suggestion.

benknoble · 2024-11-12T14:54:32Z

Ok, I confirmed that this branch worked for me using …/bin/gh pr view -w when

the defaults are left alone (so branch@{upstream} = origin/branch, where you push to)
using push.default = current (so branch@{upstream} = origin/main but branch@{push} = origin/branch)

benknoble

Just some miscellany I noticed while reading the commits.

benknoble · 2024-11-12T21:01:33Z

pkg/cmd/pr/shared/finder.go

-		progress:   factory.IOStreams,
+		pushDefault: func() (string, error) {
+			return factory.GitClient.Config(context.Background(), "push.default")
+		},
+		progress: factory.IOStreams,


It looks like progress became unaligned here, causing the diff to be noisier than needed.

I'm afraid I didn't quite follow. Could you elaborate a bit? Is it related to my PR?

Sorry, I mean the whitespace between progress: and factory changed, which is why the diff looks this way. Does go fmt realign them?

Nah it was go fmt that did this in the first place, presumably because it was separated from the lines above.

benknoble · 2024-11-12T21:03:29Z

pkg/cmd/pr/status/status.go

 		if branchConfig.Push != "" {
 			selector = strings.TrimPrefix(branchConfig.Push, branchConfig.RemoteName+"/")
-		} else if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
+		} else if pushDefault, _ := gitClient.Config(context.Background(), "push.default"); (pushDefault == "upstream" || pushDefault == "tracking") &&
+			strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
 			selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
 		}


This block is starting to look duplicated with .../finder.go's parseCurrentBranch

Don't worry, it will only get worse /s

Jokes aside, IIRC my original plan was to see if I could do a little refactoring when I got to fixing gh pr create. Now my short memory has got the best of me and I can't remember what issues gh pr create has exactly hehe. But if anyone's got some pointers in mind, feel free to share, and I can check them out when/if I find some time to get back on this :)

Sure: something to follow-up on later. Maybe I'll take a stab sometime

When using a push.default = current central workflow [1], 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] tpope/vim-fugitive#1172 (comment)

When push.default is not 'upstream' / 'tracking' (or 'nothing'), we can expect local and remote branch names to be the same and solely rely on @{push} or RemoteURL. This fixes the wrong error message 'no pull requests found for branch "<target branch>"' when the local branch is not pushed in the push.default = simple / current and upstream = <target branch> setup.

When using a push.default = current triangular workflow, apart from using @{push} to determine the remote branch name, we should also follow the 1. branch.<name>.pushRemote 2. remote.pushDefault 3. branch.<name>.remote ...list to determine which remote Git pushes to.

Frederick888 force-pushed the find-pr-by-rev-parse-push branch 8 times, most recently from cdc1b5d to 4226579 Compare June 18, 2024 00:28

Frederick888 marked this pull request as ready for review June 18, 2024 00:31

Frederick888 requested a review from a team as a code owner June 18, 2024 00:31

Frederick888 requested a review from williammartin June 18, 2024 00:31

cliAutomation added the external pull request originating outside of the CLI core team label Jun 18, 2024

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from 4226579 to c8a51dd Compare June 25, 2024 12:40

williammartin mentioned this pull request Jul 10, 2024

9208-triage #9306

Closed

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from c8a51dd to 16eed48 Compare July 11, 2024 11:48

williammartin mentioned this pull request Jul 24, 2024

pr view and pr status should respect @{push} where possible #9363

Open

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from 16eed48 to 3693b51 Compare July 25, 2024 06:01

Frederick888 force-pushed the find-pr-by-rev-parse-push branch 2 times, most recently from f247b33 to ad62216 Compare July 25, 2024 07:41

This comment was marked as spam.

Sign in to view

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from ad62216 to c55244e Compare July 30, 2024 11:31

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from c55244e to 91471ce Compare September 18, 2024 01:26

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from 91471ce to ba25124 Compare October 3, 2024 00:04

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from ba25124 to 40c2ac0 Compare October 16, 2024 23:04

Frederick888 force-pushed the find-pr-by-rev-parse-push branch 2 times, most recently from 169a789 to 776ba51 Compare November 1, 2024 02:17

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from 776ba51 to fd6c017 Compare November 10, 2024 23:42

benknoble reviewed Nov 12, 2024

View reviewed changes

Frederick888 added 3 commits November 15, 2024 08:04

Frederick888 force-pushed the find-pr-by-rev-parse-push branch from fd6c017 to 285a5df Compare November 14, 2024 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Find PRs using `@{push}` #9208

Find PRs using `@{push}` #9208

Frederick888 commented Jun 14, 2024 •

edited

Loading

williammartin commented Jul 10, 2024

williammartin commented Jul 10, 2024

williammartin commented Jul 10, 2024 •

edited

Loading

williammartin commented Jul 10, 2024

Frederick888 commented Jul 11, 2024 •

edited

Loading

williammartin commented Jul 24, 2024

Frederick888 commented Jul 25, 2024

williammartin commented Jul 25, 2024

Frederick888 commented Jul 25, 2024

Frederick888 commented Jul 25, 2024

williammartin commented Jul 26, 2024 •

edited

Loading

williammartin commented Jul 26, 2024

This comment was marked as spam.

Frederick888 commented Jul 30, 2024

williammartin commented Jul 30, 2024

Frederick888 commented Jul 30, 2024 •

edited

Loading

williammartin commented Jul 30, 2024 •

edited

Loading

Frederick888 commented Jul 30, 2024

williammartin commented Jul 30, 2024

Frederick888 commented Jul 30, 2024 via email

benknoble commented Nov 12, 2024

williammartin commented Nov 12, 2024

benknoble commented Nov 12, 2024

benknoble commented Nov 12, 2024

benknoble left a comment

benknoble Nov 12, 2024

Frederick888 Nov 13, 2024

benknoble Nov 13, 2024

Frederick888 Nov 14, 2024

benknoble Nov 12, 2024

Frederick888 Nov 13, 2024

benknoble Nov 13, 2024

Find PRs using @{push} #9208

Are you sure you want to change the base?

Find PRs using @{push} #9208

Conversation

Frederick888 commented Jun 14, 2024 • edited Loading

f9a1329f Find PRs using @{push}

a049eade Only find PRs w/ branch.<name>.merge if push.default = upstream/tracking

285a5df0 Find push remote using branch.<name>.pushRemote and remote.pushDefault

williammartin commented Jul 10, 2024

williammartin commented Jul 10, 2024

williammartin commented Jul 10, 2024 • edited Loading

williammartin commented Jul 10, 2024

Frederick888 commented Jul 11, 2024 • edited Loading

williammartin commented Jul 24, 2024

Frederick888 commented Jul 25, 2024

williammartin commented Jul 25, 2024

Frederick888 commented Jul 25, 2024

Frederick888 commented Jul 25, 2024

williammartin commented Jul 26, 2024 • edited Loading

williammartin commented Jul 26, 2024

This comment was marked as spam.

Frederick888 commented Jul 30, 2024

williammartin commented Jul 30, 2024

Frederick888 commented Jul 30, 2024 • edited Loading

williammartin commented Jul 30, 2024 • edited Loading

Frederick888 commented Jul 30, 2024

williammartin commented Jul 30, 2024

Frederick888 commented Jul 30, 2024 via email

benknoble commented Nov 12, 2024

williammartin commented Nov 12, 2024

benknoble commented Nov 12, 2024

benknoble commented Nov 12, 2024

benknoble left a comment

Choose a reason for hiding this comment

benknoble Nov 12, 2024

Choose a reason for hiding this comment

Frederick888 Nov 13, 2024

Choose a reason for hiding this comment

benknoble Nov 13, 2024

Choose a reason for hiding this comment

Frederick888 Nov 14, 2024

Choose a reason for hiding this comment

benknoble Nov 12, 2024

Choose a reason for hiding this comment

Frederick888 Nov 13, 2024

Choose a reason for hiding this comment

benknoble Nov 13, 2024

Choose a reason for hiding this comment

Find PRs using `@{push}` #9208

Find PRs using `@{push}` #9208

Frederick888 commented Jun 14, 2024 •

edited

Loading

`f9a1329f` `Find PRs using @{push}`

`a049eade` `Only find PRs w/ branch.<name>.merge if push.default = upstream/tracking`

`285a5df0` `Find push remote using branch.<name>.pushRemote and remote.pushDefault`

williammartin commented Jul 10, 2024 •

edited

Loading

Frederick888 commented Jul 11, 2024 •

edited

Loading

williammartin commented Jul 26, 2024 •

edited

Loading

Frederick888 commented Jul 30, 2024 •

edited

Loading

williammartin commented Jul 30, 2024 •

edited

Loading