scala / scabot

Scala's PR&CI automation bot
Apache License 2.0
14 stars 14 forks source link

Add ability to handle comments that ask for review #68

Closed teldosas closed 7 years ago

teldosas commented 7 years ago

Address #16

SethTisue commented 7 years ago

interesting... looks plausible... README.md would need an update... have you been able to test this?

SethTisue commented 7 years ago

nice to see some PRs coming in in this repo, btw. welcome

teldosas commented 7 years ago

have you been able to test this?

Yes I tested it in my fork of Scala

SethTisue commented 7 years ago

Yes I tested it in my fork of Scala

very cool.

@adriaanm what do you think, shall we just go ahead and merge this and #69 and try them out?

SethTisue commented 7 years ago

thought: what happens if the requested reviewer doesn't actually have write access to the repo...?

teldosas commented 7 years ago

Well the api request fails for sure, and a message is logged in the console about the user not being a collaborator. I'm not sure if it has any further side-effects. I don't think it would have. I will try it out again soon and I'll get back to you.

teldosas commented 7 years ago

Should I make it post a comment or something in that case?

SethTisue commented 7 years ago

hmmm... I suppose it's fine if it silently fails (except for the log message).

adriaanm commented 7 years ago

Awesome, thank you!! Yes, let's try this out.

SethTisue commented 7 years ago

(note that I still don't know how to deploy a new Scabot, because https://github.com/scala/scala-jenkins-infra/issues/54, and I don't think the backup method is documented anywhere)

adriaanm commented 7 years ago

The travis job for the merge commit of this PR seems to have pushed to prod. Checking if post-receive hooks worked (will continue over at the infra ticket).

adriaanm commented 7 years ago

It seems to have worked at https://github.com/scala/scala/pull/5662! I couldn't trigger a review at scala/scala#5677 myself by commenting. Not sure what's going on, as I've broken logging :-( while trying to get push-to-deploy? Will get back to this asap

teldosas commented 7 years ago

@adriaanm I think it is not possible for the commiter to be a reviewer

SethTisue commented 7 years ago

ah, the PR author.

I review-requested myself just now at https://github.com/scala/scala/pull/5677 and it worked fine.

adriaanm commented 7 years ago

Ah good point hehe! 👍 On Thu, Feb 9, 2017 at 13:23 Seth Tisue notifications@github.com wrote:

ah, the PR author.

I review-requested myself just now at scala/scala#5677 https://github.com/scala/scala/pull/5677 and it worked fine.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/scala/scabot/pull/68#issuecomment-278777845, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFjy5-DUtHGU7ly0p6FRHcLSLoUv6rfks5ra4O_gaJpZM4LyEz4 .