Skip to content

Add "changes requested" & Statuses+Checks conclusion to pr list#12

Merged
mislav merged 7 commits intoprototypefrom
pr-reviews-checks
Oct 11, 2019
Merged

Add "changes requested" & Statuses+Checks conclusion to pr list#12
mislav merged 7 commits intoprototypefrom
pr-reviews-checks

Conversation

@mislav
Copy link
Copy Markdown
Contributor

@mislav mislav commented Oct 10, 2019

  • If any of the reviewers have requested changes, display red "changes requested" next to a PR
  • Display Statuses/Checks conclusion next to each PR
  • Currently only applied to the "Pull requests created by you" section

Screen Shot 2019-10-11 at 12 07 54 AM

Ref. #4

TODO:

  • Expand approach to all PRs in pr list output
  • Resolve the conclusion of competing Statuses vs Checks
  • Present different checks conclusions in different color (e.g. "success" in green)

Adds red "changes requested" notice to PRs for which a change has been requested by any of the reviewers.
Fetches Statuses and Checks for each PR and displays information about whether the overall state is passing or failing.
@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Oct 11, 2019

New functionality:

  • resolve conclusion between multiple Statuses/Checks into a single status
  • print only failed statuses next to PRs - assumes that "success" and "pending" statuses are not actionable information
  • print failed statuses in red
  • also apply "changes requested" and Statuses information next to PR for current branch
  • tweak output alignment, widths; truncate long titles

Deliberately does not show "changes requested" nor Statuses/Checks for PRs that request your review:

  • PRs that request your review might already have reviews from other people, and one of them might be "changes requested". This information is likely not relevant to the fact that you should also leave a review.
  • These PRs might also have failing statuses. For now, I've also chosen not to display them, since these PRs are from other people who asked your review, and it's likely that those other people are responsible for fixing failing checks.

Will merge this soon since I want to use this in a demo ✨

@mislav mislav merged commit 3a80cd1 into prototype Oct 11, 2019
@mislav mislav deleted the pr-reviews-checks branch October 11, 2019 14:20
@probablycorey
Copy link
Copy Markdown
Contributor

This looks great! I agree with everything but have some doubts about the "print only failed statuses" part. Since pending and success will be indistinguishable, I'm thinking that pending is actionable because otherwise I'd just assume my PR changes passed CI. Even though the action is "I guess I'll wait until it finishes" I'd still want to know that bit of information.

mislav pushed a commit that referenced this pull request Sep 28, 2021
Stonre pushed a commit to Stonre/strands-fork that referenced this pull request Jul 21, 2025
Co-authored-by: Patrick Gray <pgrayy@amazon.com>
Co-authored-by: Jason Kim <jkim@anthropic.com>
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.

2 participants