search

servo / highfive

Github hooks to provide an encouraging atmosphere for new contributors
Mozilla Public License 2.0
254 stars 58 forks source link

Asignee vs “you should hear from” #83

Closed SimonSapin closed 8 years ago

SimonSapin commented 8 years ago

highfive randomly assigns new PRs to a reviewer. If the PR is by a new contributor, highfive will separately pick a reviewer randomly and make a message like

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from […] (or someone else) soon.

If I’m picked for the latter but not the former, it’s not clear to me if I should do anything or leave it to the assignee to deal with that PR. Perhaps highfive should always pick the same person for both?

jdm commented 8 years ago

Yeah, this will require either merging the handler that sets the assignee with the welcome one, or add some way of coordinating choices between them. Merging them is probably easier.

zbynekwinkler commented 8 years ago

I will fold the welcome one into the other. The message confused me too when I saw it :smiley:

jdm commented 8 years ago

Thanks!

zbynekwinkler commented 8 years ago

Hmm. I think the code causes me a physical pain :wink:. The handlers appear to be by all signs separate modules but they are imported into one shared module called __init__ and even make use of it (like the call to get_collaborators in handlers/welcome/init.py#L10). Was that the intention? I hope you don't mind if I first split them up.

wafflespeanut commented 8 years ago

The only change that will be required is to merge the on_pr_opened functions from WelcomeHandler and AssignReviewerHandler. If the PR is from a new contributor, then we should get the assignee (instead of choosing one randomly), and post the welcome message. This will render the welcome handler useless, and so we should finally remove it.

wafflespeanut commented 8 years ago

90 should fix this