[RFE] PRs should support add_reviewer action

[chessbyte]

See ManageIQ/manageiq#14552 (comment) for example usage.

[moolitayer]

cc @abonas

[abonas]

I also wondered about this ability recently

[NickLaMuro]

From @chriskacerguis 's comment in the duplicate issue:

Question...should there be some kind of inverse for this?

@miq-bot remove_review @username
@miq-bot un_review @username ???

I think this is something that really doesn't exist even with the assign functionality, and has a bit of a permission issue involved with it. I would say that if we were to do this, you could only remove yourself, and otherwise this is something that the project owners already have access to do.

[Fryguy]

There is a feature for dismissing reviews that only those with merge rights can do, but it requires a reason. So it we wanted to expose that, the command would have to be something like

@miq-bot remove_reviewer @Fryguy This is my reason for removing

I would say that if we were to do this, you could only remove yourself, and otherwise this is something that the project owners already have access to do.

I agree.

[chrisarcand]

I don't think there really needs to be an inverse at all. Designating a reviewer is requesting a review from someone; how common do you think it is for people to 'unrequest' someone to review something? It usually involves just requesting someone else anyway, if that person is gone.

I'd just do @miqbot r? @username (and/or request_review), which adds users to the reviewers list, and not go through the trouble of permissions for removing them as the exceptions are few and maintainers (write access) can handle that 2% case.

[NickLaMuro]

I don't think there really needs to be an inverse at all.

@chrisarcand I would argue there is a use case, especially for those who are asked to review, but then feel like they don't have the proper skill set to review. As an example:

ManageIQ/manageiq#14525 (comment)

In this case, Greg had permissions to remove himself, but for folks like myself, if I were assigned as a reviewer (right now, by someone who has merge rights), I have no way of removing myself. This would give those without merge rights the ability to opt out of a review they were asked to do that they don't feel like they were qualified to do in the first place.

It usually involves just requesting someone else anyway, if that person is gone.

Sure, this is fine I guess, but I would say there have been many times where I have had 4 or so reviewers stacked up on a single PR, and it would be in deadlock trying to determine who had the final say since maybe only 1 had given approval, and the rest are either in limbo or not responding.

[chrisarcand]

¯\_(ツ)_/¯ Fair. Then for that use case, the command could be further simplified by not having a generic remove_reviewer @username to remove anyone, and having decline_review or whatever instead which only removes the person that gave the command. Maintainers, who have write access are the only ones who should be removing people that aren't themselves anyway(?)

(Realized this is a longwinded way of saying yes to the above comments, just don't bother having to include a username as it always has to be you...)

[NickLaMuro]

Agreed, I think the decline_review suggestion makes more sense in this case, and agree that the "inverse command" probably should be just for the person who wants to remove themselves from the review process, and not require you to designate who you are trying to remove.

[chessbyte]

See https://developer.github.com/v3/pulls/review_requests/ for GitHub API

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

[Fryguy]

Closed via #408 and #419