ejoffe / spr

Stacked Pull Requests on GitHub
MIT License
796 stars 68 forks source link

0.12.2 does not work with repos with a lot of pull requests #319

Closed tgeng closed 1 year ago

tgeng commented 1 year ago

First I want to thank you for creating this! I have been using this daily for my work since version 0.9 and it has been great!

Recently I upgraded to 0.12.2 and it seems the new way of pulling PRs does not work very well for repos with a lot of PRs open. I kept getting errors like the following

 Date: Fri May 12 14:26:27 2023 -0700
 6 files changed, 70 insertions(+), 23 deletions(-)
> git rev-parse --show-toplevel
> git fetch
> git branch --no-color
> git rebase origin/master --autostash
> github fetch pull requests
> git branch --no-color
> git branch --no-color
> git log --format=medium --no-color origin/master..HEAD
> git rebase origin/master -i --autosquash --autostash
> git log --format=medium --no-color origin/master..HEAD
> git branch --no-color
> git branch --no-color
> git log --format=medium --no-color origin/master..HEAD
> git status --porcelain --untracked-files=no
> git push --force --atomic origin 48cf23e2eda59f3348198ae2f16942eade3e69ae:refs/heads/spr/ed125cd8
> git branch --no-color
> github create 9387 : Make import under try optional
> github update 9387 : Make import under try optional
> git branch --no-color
> github fetch pull requests
> git branch --no-color
> git branch --no-color
> git log --format=medium --no-color origin/master..HEAD
> git branch --no-color
pull request stack is empty

Is it possible to bring back the old way of retrieving PRs? Or maybe control it with some flag?

BTW it would be great if spr can ignore unrecognized flags in .spr.yml under a repo. Otherwise, when new flags are added, people using old versions of spr are either forced to upgrade or force git to ignore changes to .spr.yml. Neither is great.

tgeng commented 1 year ago

Also, sometimes git spr update fails with

> git rev-parse --show-toplevel
> git fetch
git error: error: cannot lock ref 'refs/remotes/origin/master': is at c2b5bd07d8dba653b26a6858a2fb40e9815be222 but expected 3d26e34b51496570a2b633b6f66c40bad635160f
From github.com:foo/bar
 ! 3d26e34b5149..c2b5bd07d8db  master     -> origin/master  (unable to update local ref)
panic: exit status 1

goroutine 1 [running]:
github.com/ejoffe/spr/git/realgit.(*gitcmd).MustGit(0x101000000000002, {0x88ee68, 0x0}, 0x8)
        /Users/runner/work/spr/spr/git/realgit/realcmd.go:44 +0x45
github.com/ejoffe/spr/spr.(*stackediff).fetchAndGetGitHubInfo(0xc0000b9020, {0x92c590, 0xc0000a8000})
        /Users/runner/work/spr/spr/spr/spr.go:468 +0x8d
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc0000b9020, {0x92c590, 0xc0000a8000}, {0x0, 0x4, 0x0}, 0x0)
        /Users/runner/work/spr/spr/spr/spr.go:119 +0xa5
main.main.func4(0xc0000d8b00)
        /Users/runner/work/spr/spr/cmd/spr/main.go:155 +0x114
github.com/urfave/cli/v2.(*Command).Run(0xc0001ac900, 0xc0000bb3c0)
        /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/command.go:163 +0x64a
github.com/urfave/cli/v2.(*App).RunContext(0xc0001b6340, {0x92c590, 0xc0000a8000}, {0xc0000bc000, 0x2, 0x2})
        /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:313 +0x81e
github.com/urfave/cli/v2.(*App).Run(...)
        /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:224
main.main()
        /Users/runner/work/spr/spr/cmd/spr/main.go:224 +0x1325
ejoffe commented 1 year ago

Regarding the first issue, I don't actually see an error in the log. It does look like it updated a pr and the resulting list is empty, but were there any errors?

ejoffe commented 1 year ago

The second issue might be related to the remote branch being "main" and not "master", this should be fixed in the upcoming version (v0.13).

tgeng commented 1 year ago

So the above two output are actually fit the same branches. Typically what happens if that I see the second error upon spr update. If I try again I see the first output. But a pr is successfully created. However, due to the issues of pr discovery, further spr operations won’t work.

ejoffe commented 1 year ago

How many open pull requests do you have in the repository?

ejoffe commented 1 year ago

I looked at the code and github api again. Unfortunately it doesn't look like the api has the ability to filter repository pull requests. Currently the code gets the first 100 pull requests and doesn't have any logic for pagination (getting the rest). If you have more than 100 open pull requests this might break the code and possibly be the root cause of this issue. Depending on how many you have open, we can try to increase this limit a little and see if that works. Otherwise would have to implement pagination in the client.

tgeng commented 1 year ago

Thanks for looking into it! The repo has hundreds (up to a thousand maybe) open PRs (it’s a monorepo hosting many different projects used by hundreds of people).

Instead of querying open PRs in a repo, is it possible to query open PRs from a person? It seems unlikely that a single user can have so many open PRs.

ejoffe commented 1 year ago

Fixed by @Delerme