Skip to content

feat: fetch all/more reviewers + show “suggested” (code owner) reviewers too#728

Merged
dlvhdr merged 3 commits intodlvhdr:mainfrom
sideshowbarker:main
Jan 3, 2026
Merged

feat: fetch all/more reviewers + show “suggested” (code owner) reviewers too#728
dlvhdr merged 3 commits intodlvhdr:mainfrom
sideshowbarker:main

Conversation

@sideshowbarker
Copy link
Contributor

@sideshowbarker sideshowbarker commented Jan 2, 2026

Summary

This change implements two follow-ups in response to #724 review comments:

#724 (comment)
#724 (comment)

Specifically:

  • Increases # of reviewers limit from last 5 reviewers to 100 reviewers
  • Shows “Loading…” state while enriched data is being fetched
  • Shows “suggested” reviewers (= code owners), even if not yet requested
  • Shows also those reviewers who only commented (not just approved/rejected)

How did you test this change?

Updated internal/tui/components/prview/reviewers_test.go with new tests.

dlvhdr
dlvhdr reviewed Jan 2, 2026
View reviewed changes
Copy link
Owner

@dlvhdr dlvhdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Just small comments

@sideshowbarker sideshowbarker force-pushed the main branch 2 times, most recently from 6e48078 to d7c50b5 Compare January 2, 2026 22:03
@sideshowbarker
feat: fetch all/more reviewers + show "suggested" (code owner) review…
cd82ed7 
…ers too

Move ReviewRequests and Reviews from PullRequestData to EnrichedPullRequestData,
increasing the limit from 5 to 100. Show a "Loading…" state while enriched data
is being fetched.

Show reviewers who only left a COMMENTED review (not just APPROVED or
CHANGES_REQUESTED).

Show suggestedReviewers (code owners who haven't been requested yet) via
GitHub's GraphQL API, displayed in faint style with the owner icon.
@dlvhdr
Copy link
Owner

dlvhdr commented Jan 3, 2026

@sideshowbarker I did a bit more testing and it seems there's a small bug.
When reviewers leave several reviews we show the last one's state. This can lead to a wrong conclusion.
For example, if a reviewer approved a PR, then did another review with a "COMMENT" state, we will show the comment icon instead of the check mark icon.
The solution is that we shouldn't override existing review states with a COMMENT state.
Also, do we need to make sure the reviews are ordered by creation date or does that already happen?

PR for example: charmbracelet/bubbles#639

@sideshowbarker
Copy link
Contributor Author

sideshowbarker commented Jan 3, 2026

@sideshowbarker I did a bit more testing and it seems there's a small bug. When reviewers leave several reviews we show the last one's state. This can lead to a wrong conclusion. For example, if a reviewer approved a PR, then did another review with a "COMMENT" state, we will show the comment icon instead of the check mark icon. The solution is that we shouldn't override existing review states with a COMMENT state.

hmm yeah, you’re right — when we are looping there, we just need to continue for any COMMENT we reach when the previous state is a non-comment. So I will make it do that

Also, do we need to make sure the reviews are ordered by creation date or does that already happen?

Pretty sure that is how they are ordered already. I think the only problem with that is, for a comment which in creation order comes in after a non-comment, we just want that to be a no-op (like discussed above). But for anything else, we do want to just take them as-is as they are — in creation-date order,

But I will test it and make sure.

sideshowbarker added a commit to sideshowbarker/gh-dash that referenced this pull request Jan 3, 2026
@sideshowbarker
fix: don't let COMMENTED review override APPROVED or CHANGES_REQUESTED
fa42352 
Make a COMMENTED state never override an APPROVED or CHANGES_REQUESTED
state, for the purposes of the state indicator we show in Reviewers.
Make the indicator stay at the most-significant state.

Fixes dlvhdr#728 (comment)
@sideshowbarker
Copy link
Contributor Author

sideshowbarker commented Jan 3, 2026

The solution is that we shouldn't override existing review states with a COMMENT state.

Made it so.

Also, do we need to make sure the reviews are ordered by creation date or does that already happen?

So after looking and thinking, I believe we’re doing the right thing — with ordering y UPDATED_AT. Treating the “follow up” comments as more-significant than the actual actions just had kind of thrown a wrench in that. But with the follow-up-comment handling made right now, I think the basic handling/ordering is how we want it to be.

@sideshowbarker sideshowbarker requested a review from dlvhdr January 3, 2026 20:56
@sideshowbarker
fix: don't let COMMENTED review override APPROVED or CHANGES_REQUESTED
7bfcd43 
Make a COMMENTED state never override an APPROVED or CHANGES_REQUESTED
state, for the purposes of the state indicator we show in Reviewers.
Make the indicator stay at the most-significant state.

Fixes dlvhdr#728 (comment)
dlvhdr
dlvhdr approved these changes Jan 3, 2026
View reviewed changes
Copy link
Owner

@dlvhdr dlvhdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! thx again for the work!

@dlvhdr dlvhdr merged commit 50f3fcc into dlvhdr:main Jan 3, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dlvhdr dlvhdr dlvhdr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sideshowbarker @dlvhdr