search

chevah / github-hooks-server

Handling of GitHub hooks for Chevah project
BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

Remove reviewer after a "changes-approved"/"needs-changes" command from them #35

Closed danuker closed 1 year ago

danuker commented 2 years ago

In this comment, @dumol commented "changes-approved" but did not make a GitHub "review".

Not sure why I'm still listed as pending reviewer. Trying it the GitHub way…

Originally posted by @dumol in https://github.com/chevah/server/pull/5580#pullrequestreview-904043376

See whether it is possible to remove a person from a PR's "Reviewers" field, after a changes-approved or needs-changes command in a comment.

Do NOT remove an actual review containing "changes-approved" or "needs-changes", just the user from review-requests, if the command was in a COMMENT and GitHub is awaiting a review.

adiroiban commented 2 years ago

So. if the reviewer has already left a "Request changes" "GitHub Review" action, you can't remove it from the list of reviewer. That reviewer will need to use the GitHub Review UI and approve the change.

That is , you can't mix normal comments with "GitHub Review UI".

adiroiban commented 2 years ago

I think that for PRs we should stop supporting the "changes-approved" keyword. Instead, one should use the GitHub UI.

The "changes-approved" keyword still makes sense for issues.

Note that for our work, we have some work items that don't have a PR associated... but we track the progress and state of that issue in a similar way to a PR.

adiroiban commented 2 years ago

@danuker

So, "changes-approved" should not be used for PR. Instead, use the GitHub UI button.

So, I think that this should be closed as won't fix.

danuker commented 1 year ago

I ran into this here. But I also think we should not fight the GitHub UI, and we should drop support for changes-approved in PRs.