lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard
verified publisher icon jesseduffield

WIP: Show pull requests against branches

Open jesseduffield opened this issue 2 years ago • 5 comments

  • PR Description

If the user has gh installed and it's on version 2 or greater, we'll refresh pull requests at the same time we run a background git fetch.

There are a lot of touch points here:

  • gh is written in Go and we could avoid an external dependency by using the gh code directly, however doing so would mean taking on the burden of handling things like authentication, which would increase the scope quite a bit. Given that pulling pull requests takes a while (3.5s for me), the extra cost of shelling out to gh is negligible in comparison
  • There's no way to say 'only fetch the PRs for specific branches' so we instead pull the latest 500 PRs (looking at the code this may be limited to 100 anyway). This unfortunately takes some time. Filtering to only open PRs makes no difference.
  • We show PRs by default, but it can be opted out of
  • If the user does not have gh installed we won't attempt to fetch anything
  • If the user has gh installed but on an old version (<2) we log a message to the user and don't attempt to retry
  • If there is an authentication failure we log and then retry later on
  • We refresh PRs at the same rate as we do a git fetch
  • If the user hasn't configured the base repo of the current git repo, we prompt them to do so.
  • Currently we determine that we're in a git repo by looking at the url of the first remote, to see whether it contains github.com. Gh itself looks at the configured hosts file to see if there is an authentication setup for any host. We should do the same.
  • Please check if the PR fulfills these requirements
  • [ ] Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
  • [ ] Code has been formatted (see here)
  • [ ] Tests have been added/updated (see here for the integration test guide)
  • [ ] Text is internationalised (see here)
  • [ ] Docs (specifically docs/Config.md) have been updated if necessary
  • [ ] You've read through your own file changes for silly mistakes etc

jesseduffield avatar Jul 15 '23 05:07 jesseduffield

Oooh, very nice!

A few thoughts after trying it (very briefly though, so I might be missing a lot of things):

  • It looks like it's fetching the PRs only after it's done fetching the remote(s). Is there a reason for this? For me, fetching often takes a lot of time, so I have to wait quite a while until the PR numbers show up.
  • I'm not sure the 500 limit works well. For our monorepo at work this amounts to roughly one month; sometimes we have PRs open for longer than that, so some of my open branches don't show PR numbers. Also, it would be really nice if you would also see PR numbers for very old branches. I haven't thought at all about how to achieve that, so sorry if this is not very constructive input.
  • It looks like PR numbers are only shown in the local branches panel. I'm thinking they could be even more useful in the remote branches list. (Not totally sure about that, it's just a feeling for now.)
  • Do I understand it right that at the moment the only thing this does is add the PR numbers to the display? I somehow expected you would be able to do more with this; the obvious wish is to open a PR in the web browser (I have a custom command for this). Maybe the o command could be overloaded for this? Show the existing PR if we know there is one, and open a new one if not.

stefanhaller avatar Jul 15 '23 09:07 stefanhaller

It looks like it's fetching the PRs only after it's done fetching the remote(s). Is there a reason for this? For me, fetching often takes a lot of time, so I have to wait quite a while until the PR numbers show up.

It needs remotes to map from PRs back to branches i.e. the PR has an owner e.g. 'jesseduffield') and the remote has a URL (e.g. github.com/jesseduffield/lazygit) and the branch has the name of the remote. So we need to piece those together to know that a particular PR should be assigned to a particular branch. imo getting remotes should be really fast because it's just a matter of reading a file so if it's currently a bottleneck it's worth looking into.

I'm not sure the 500 limit works well. For our monorepo at work this amounts to roughly one month; sometimes we have PRs open for longer than that, so some of my open branches don't show PR numbers. Also, it would be really nice if you would also see PR numbers for very old branches. I haven't thought at all about how to achieve that, so sorry if this is not very constructive input.

No need to apologise, I've been thinking the same thing. I wish there was a way to pass multiple branch heads to gh or to the graphql endpoint (gh is just a way of talking to graphql) but from what I've seen and tried, there's not. We could try a parallelised approach of shelling out to a bunch of gh processes at once but it would probably be rate limited.

It looks like PR numbers are only shown in the local branches panel. I'm thinking they could be even more useful in the remote branches list. (Not totally sure about that, it's just a feeling for now.)

I agree, and it shouldn't be hard to support that.

Do I understand it right that at the moment the only thing this does is add the PR numbers to the display? I somehow expected you would be able to do more with this; the obvious wish is to open a PR in the web browser (I have a custom command for this). Maybe the o command could be overloaded for this? Show the existing PR if we know there is one, and open a new one if not.

Yep currently it's just for display but I like your idea.

jesseduffield avatar Jul 15 '23 12:07 jesseduffield

Is there a plan to make gh pr usable in LG, or only the external view? I would love to see it allow us to use 'gh pr create/edit/etc'.. if lg could add in support for the gh command for prs... it would be amazing and make it so i dont even have to leave my nvim/lg setup xD

Daemoen avatar Jul 10 '24 00:07 Daemoen

@Daemoen You can easily do this as a customCommand. For example, for creating a PR on the current branch:

customCommands:
  - key: "o"
    command: "gh pr create"
    subprocess: true
    context: "localBranches"

mpasa avatar Jul 12 '24 06:07 mpasa

Could we show a list of pull requests and their titles? I would actually use this instead of the branch view most of the time. This way I wouldn't need to remember the name of the branch or the PR number. I just need the (human readable) title.

This would also be useful for code review. Currently I have to go to GitHub, copy the branch name, and checkout the branch, whereas with this workflow I could just directly checkout the PR inside of lazygit. The branch name is an implementation detail.

OliverJAsh avatar Aug 09 '24 08:08 OliverJAsh