Miss leading "Changes requested" status when a "Review Request" has been requested after applying changes. #17875
Replies: 6 comments 6 replies
-
|
Reviewers already get a notification in https://github.com/notifications Maybe the UI could have a counter |
Beta Was this translation helpful? Give feedback.
-
|
Yeah, having the count would be awesome as well! 😀 The problem is not about notifications, but about being able to read the PR list, and quickly see which PR has changes requested, which is approved, and which is still waiting. |
Beta Was this translation helpful? Give feedback.
-
|
Agreed with davola – in our organization we have some people who like to work based on notifications, and some people who like to work from the list. For those using the list and just seeing "Changes Requested" instead of "Review Required" or "Review re-requested", it instantly feels like the ball is with the author, while it is actually with the reviewer. |
Beta Was this translation helpful? Give feedback.
-
|
100% agree, this drives me crazy! Yea, i get a github notification, but that is not helpful, cos those don't even properly get pushed to teams in any sensible matter. This list should show what the current status is.... If Changes are requested.... and then complete, and the committer has hit the Re-Request Review button... the status in this list should show Review Requested, or Re-Review Request, NOT changes requested. that is not where the PR is at, I should be able to see, at a glance of this list, which ones are ready for me to take another look at. |
Beta Was this translation helpful? Give feedback.
-
|
IMO this is a bug that breaks my PR review workflow. Solving this (compared to other accepted asks) seems relatively simple but would significantly improve the PR review workflow. |
Beta Was this translation helpful? Give feedback.
-
|
agreed 100% |
Beta Was this translation helpful? Give feedback.
-
|
Agreed. The current flow makes you very dependent on using notifications to know which of your opened PRs need action after a subsequent review or if they're still waiting. Or constantly opening all your open PRs to see their real status. As a work-around I've now resorted in some of my repositories to run a GitHub actions script that dismisses existing reviews when re-requesting, since that then updates the status back to 'review required'. |
Beta Was this translation helpful? Give feedback.
-
|
any chance you could post that script? it would be very helpful. |
Beta Was this translation helpful? Give feedback.
-
|
I have this defined in a separate "github actions workflows" repository, so that I can re-use the same action from multiple repositories: name: Dismiss old reviews when re-requested
on:
workflow_call:
jobs:
review_requested:
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- name: Dismiss old reviews
uses: actions/github-script@v7
with:
script: |
const reviews = await github.paginate(github.rest.pulls.listReviews, {
pull_number: context.payload.number,
owner: context.repo.owner,
repo: context.repo.repo,
});
for (const review of reviews) {
if (review.state === 'CHANGES_REQUESTED' && review.user.id === context.payload.requested_reviewer?.id) {
console.log('Dismissing re-requested review.');
await github.rest.pulls.dismissReview({
pull_number: context.payload.number,
owner: context.repo.owner,
repo: context.repo.repo,
review_id: review.id,
message: 'Review re-requested',
});
}
}Then I call it with the following workflow file that lives in each repository that needs it: name: Dismiss old reviews when re-requested
on:
pull_request:
types: [review_requested]
jobs:
review_requested:
permissions:
pull-requests: write
uses: stianjensen/custom-github-actions-workflows/.github/workflows/dismiss_old_reviews.yml@main |
Beta Was this translation helpful? Give feedback.
-
|
Thank you! I cant say I expected an answer that quickly! This could be very useful! |
Beta Was this translation helpful? Give feedback.
-
|
The user who has reviewed the code and requested for changes has gone on leave so what would happen for the PR - |
Beta Was this translation helpful? Give feedback.
-
|
push, most annoying feature right now |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
A given repo could have several PRs, on which their statuses can be "approved", "changes requested" or "Re-Review requested" (when changes are submitted and a new review is requested)
This third status where the PR has a pending review request is not reflected properly on the PR list status.
Consider this scenario or flow:
When a PR has a Review submitted with some changes pending, this is properly displayed on the PR list as "Changes Requested" as shown below
After the user applies those review changes requested, and submits the PR back for another review, the PR status keeps showing the wrong "Changes Requested" to the reviewer.
You can check so in the image below. The filter for show only the PR that have a pending review for the user is set, but all the PR on that list appears to be on the status "Changes Requested", when it should say something like "Review Re-Requested" to reflect actual the actual status of it.
This will allow any reviewer to read the full list, without the need of applying any filter, and still be able to understanding which of the PRs there are ready for him to take action and start another review round.
Hope this make sense for anybody else, and thank you for your time on reading this! 🙏
Beta Was this translation helpful? Give feedback.
All reactions