Skip to content

Issue Frecency - #4768

Closed
pxrth9 wants to merge 26 commits into
trunkfrom
issue-frecency
Closed

Issue Frecency#4768
pxrth9 wants to merge 26 commits into
trunkfrom
issue-frecency

Conversation

@pxrth9

@pxrth9 pxrth9 commented Nov 19, 2021

Copy link
Copy Markdown
Contributor

Frecency Epic

This PR implements the frecency algorithm ( Ref. #4754, #4753) for suggesting open issues and pull requests.
Frecency stats are stored in a local sqlite database and regularly synchronized.

To-Do List

  • database CRUD
  • database pruning
  • issue frecency
    • view
    • edit
    • close
    • delete
    • comment
  • PR frecency (right now gh pr commands use the current PR with no args)
    • view
    • edit
    • close
    • ready?
    • review?
  • frecency clear command
  • tests
    • frecency manager mocking
    • test CRUD
    • test API calls
    • fix issue/PR tests

@meiji163

meiji163 commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Notes:

  • The current strategy to keep the database updated is to fetch new issues since the last query each time frecency is used. Periodically, stale records are pruned by dropping all issues with frequency count < some threshold. Then, the status of the remaining issues are queried to check if they are still open.
  • One UX concern is keeping the query times fast (<1s). The above strategy seems to be fast enough because there will likely be a small number of issues with a significant count, and fetching the first page of issues is fast.
  • We mostly abandoned the idea of easily generalizing to other datatypes, in favor of specializing to issues and PRs. Since repositories are already in the SQL table, they may be the easiest to add frecency to next.

@meiji163

meiji163 commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

More Notes:

  • We currently use the graphQL search API for pull requests because there is no filterBy option for graphQL PullRequests. It can be slow for large repositories (e.g. pytorch/pytorch with 3k+ open PRs). It may be necessary to change PR lookups to REST.
  • adding frecency to PRs is more complicated than issues, since PRs are looked up with the PRFinder in pkg/cmd/pr/shared

@vilmibm

vilmibm commented Jan 5, 2022

Copy link
Copy Markdown
Contributor

This is awesome work; I've been playing with it locally and it feels very nice. The performance of gh pr view<enter><enter> on a cold cache is not perceptibly slower than directly running gh pr view 1234 (on average, about 130ms in the former and 90ms in the latter). I tested against github/github.

To me, the huge and obvious risk here is adopting sqlite. It's a large surface area of code to maintain and a vector for bugs and state issues. I'm not entirely opposed to adopting it for this purpose as I think the data is so ephemeral that we can safely destroy and remake the sqlite file as needed without having gh blow up. I am totally sympathetic to @samcoe and @mislav though if they have reservations about maintaining sqlite-involving code going forward.

Coming back from vacation and thinking about this with fresh eyes, there is a potentially much smaller code footprint version of this UI concept that I'd like to take a stab at prototyping on top of this PR: populating the frecency list only with explicit arguments that users give gh over time. In other words, it doesn't fully alleviate having to manually type or paste a pr/issue number, but saves repeated entering of pr/issue numbers. It'll be easy for me to prototype this given the frecency manager work in this PR.

@samcoe samcoe added the discuss Feature changes that require discussion primarily among the GitHub CLI team label Apr 7, 2022
@samcoe samcoe removed the discuss Feature changes that require discussion primarily among the GitHub CLI team label Apr 20, 2022
@mislav

mislav commented Jun 14, 2022

Copy link
Copy Markdown
Contributor

See resolution in #4754 (comment)

@mislav mislav closed this Jun 14, 2022
@samcoe
samcoe deleted the issue-frecency branch June 28, 2023 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants