Responding to GitHub's review events

[pwaller]

Could we have Homu respond to GitHub's review feature, as well as r+? The event is called PullRequestReviewEvent.

[Manishearth]

This would have to be a setting. Often folks use the GitHub review thing to say "approved, but here are some minor changes you should make and then self-r+"

…

On Wed, Oct 10, 2018, 5:06 AM Peter Waller ***@***.***> wrote: Could we have Homu respond to GitHub's review <https://help.github.com/articles/about-pull-request-reviews/> feature <https://blog.github.com/2016-12-07-introducing-review-requests/>, as well as r+? The event is called PullRequestReviewEvent <https://developer.github.com/v3/activity/events/types/#pullrequestreviewevent> . — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#173>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSOILSJeXYusPaB4jnHf1wQJAtX9yks5ujeK3gaJpZM4XVMQY> .

[pwaller]

Ah, that is a fair point, I guess you can't self-review on github. Hmm.

[pwaller]

On the other hand, you can always use the r+ feature in conjunction with this, rather than instead-of.

[Manishearth]

Right. We should definitely pick up bors commands from review events. We should not interpret reviews events themselves as bors commands

…

On Wed, Oct 10, 2018, 8:32 AM Peter Waller ***@***.***> wrote: On the other hand, you can always use the r+ feature in conjunction with this, rather than instead-of. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#173 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSF-7LPDpCbPxkbdccO0Y5LpsZhlYks5ujhLvgaJpZM4XVMQY> .

[pwaller]

I'd like to have a setting that does so. If a reviewer wants to give feedback and 'passively approve' (i.e, allow the submitter to self-r+), they can always leave a 'comment review' rather than 'approve/disapprove'.

[kennytm]

Often folks use the GitHub review thing to say "approved, but here are some minor changes you should make and then self-r+"

The r+ command is only interpreted you mention @bors on the same exact line, so I think this cause of misapproval would be rare.

[Manishearth]

You misunderstand, the proposal is (in part) to make an approved review itself count as an r+

[pwaller]

The r+ command is only interpreted you mention @bors on the same exact line, so I think this cause of misapproval would be rare.

My interpretation of @Manishearth's comment was that pressing "Approve" in github should not translate to the equivalent of r+, because people currently use "Approve Review" in github and tell a user to self-r+ to cause a merge.

[kennytm]

Okay my understanding was that homu should respond to review comments (which are currently ignored).

I agree that pressing "Approve" should not be equivalent to r+, without mentioning @bors the intention is not clear enough.

[pwaller]

I would like to try and encourage my team to use the "Approve" feature, to give it an equivalence to r+. Using r+ (e.g, self-r+) would also be allowed. So it would be nice if this were an option that could be enabled.

My motivation is to get people to use the github review feature, for example to request reviews, and also to give them a nudge UI nudge towards leaving comments using the review feature (which is only seen on the 'diff' section).

[Manishearth]

Yeah, there are two separate features here:

• GH approval comments should definitely be parsed for homu commands
• GH approvals should themselves count as an approval, provided an option is set

I don't really have time to implement this but willing to accept a patch for either.