Use Mention Bot to Promote Review of Pull Requests

[sunjay]

cc @PistonDevelopers/admins @PistonDevelopers/pistoncollaborator (Sorry for mass mentioning everyone)

Mention Bot: https://github.com/facebook/mention-bot#readme

There is a TL;DR below.

Overview

Mention bot analyses git blame information and chooses appropriate people to mention whenever a pull request is created.

Since Piston is a large, distributed project with many contributors, most people are not particularly active. They add what they can, and the rest is left to a very small minority of frequent collaborators. Many people contribute and open pull requests for their code. However, with our limited resources, it's still very common to see pull requests get left unreviewed and unmerged for a long time.

The way we deal with this now is by encouraging people to merge in their own pull requests. This works sometimes, but it's hard to know if your changes will break someone else's work. It gets worse because sometimes when people do self merge, they break the code from the people before them. New contributors who don't know the code well don't have an easy way to mention relevant people and get their code merged.

Mention bot will figure out who the appropriate people are for each pull request, and let them know that a pull request was created.

Rest assured, this won't actually change any existing process. People can still merge their own pull requests. Mention bot is just a system to let people know what is going on and hopefully get piston collaborators to interact with each other more.

No one is obligated to review anything. Mention bot will just let you know and you can choose whether you want to do anything or not.

I contacted Sven (@bvssvni) and we agreed that we should get the other piston developers involved before this kind of decision is made. Piston's projects are bigger than any one of us, so it is important that we gain some consensus before we try this out.

TL;DR:

Pros

• Mention bot will mention relevant people in each new pull request
• You will be notified if someone modifies code you probably know about
• More people will be encouraged to review open pull requests which means fewer pull requests left open and unmerged
• Pretty much everything is the same as before except for the addtion of these new mentions
• We can always turn this off if it becomes annoying
• You can always unsubscribe if you do not want mention bot to mention you again

Cons

• For more active contributors, this might result in a lot of emails (you can turn this off in your GitHub settings)
• This still may not get people to review and merge pull requests

Rollout Plan

I suggest we add this to every piston repository as soon as we gain some consensus. If people really don't like it, we can turn it off very easily. This will only act on new pull requests, so we don't have to worry about people getting tons of emails from it retroactively going through everything from before.

Discussion

What do you think? Can we go ahead and add mention bot to all of the repositories? Do you think the piston projects you collaborate on would benefit from this?

[nwin]

Especially given limited time available this could also be contraproductive.

Just imagine something gets assigned to someone who doesn't have time to review. Then it would lay there forever because nobody cares as somebody else is assigned to it.

[bvssvni]

I like that people can self merge, then I go look at what they did after receiving an email when I get time. The point with the system, as I understood it, is to get smarter notifications. If people continue to self merge, then it will not change anything in the process, but there might be some better info for people who like to check what happened yesterday, or last week.

Some points to consider:

• Do people wonder what will happen to their PR? (the usual answer might be that an active contributor is busy or need to prioritize something that is under complex planning)
• What else can we do to fix the PRs that have been waiting for days/months? (I usually work in short periods on a project with long periods in between, this is when I "clean up" old issues)
• Some other people might want better control to target the projects and code they are interested in (personally a new system would not help me much, since I receive an email for every PR)
• Periods of stabilization of a library might benefit from more reviews

Link to self merging analysis: PistonDevelopers/piston#674. It is simply faster to fix mistakes on average, and while reviews seems more secure against some bad decisions, it eats up more time on average. Is it still true today? I think there is an assumption here, something like "it is nowhere near finished anyway".

[sunjay]

@nwin @bvssvni

I may have overstated the impact this will have on code reviews. That's something I see improving further down the line. You're right that it's time consuming to review everything in detail, that's why I see this more as a notification method than something to force people to review every pull request. Mention bot doesn't assign anyone to anything. It just helps to automatically pick people who may be interested in the code being submitted. People can still self merge as usual.

This is something that could benefit both existing contributors and new contributors as more people than just the two of you start to contribute more often. Right now it might be reasonable for you to look at every pull request, but as other people start to take more ownership, having something like this in place will definitely come in handy.

For existing contributors, this adds a way for us to be notified when someone changes code that we worked on.

For example: I contribute very infrequently, but I would definitely like to know if someone is modifying the image functions I updated. Watching the repo doesn't make sense for me because I don't know about much else in the code. I don't need to be around for every pull request.

For people who contribute more often than me or want more control, they can continue to watch the repos as normal. As the project gains more contributors, this bot will give you a better idea of which pull requests to pay extra attention to. You can even setup mention bot to always notify you on a repo-by-repo basis.

For brand new contributors, this let's you know who to contact if you're not sure about what you're doing. Some of piston's repos are quite big so it's natural for someone who is just around to fix a small bug to be unsure about how their change will effect the rest of the code. With this, at least they know who to talk to for help before they just take a chance and self merge.

If someone doesn't want to get mentioned, they can always unsubscribe from mention bot. If someone who gets mentioned is being asked to review and doesn't want to or doesn't have time to, they can always just say that in the pull request.

Mention bot has a bunch of useful options too. We can modify the message to include that the same self merge rules apply.

Finally, given how easy it is to turn this on or off, we can always turn it on and then remove it if it turns out to be helpful. That's why I'm suggesting that we turn it on everywhere at once. The best way to know if this makes any sense is to try it out for a few weeks and see what happens.

[HeroicKatora]

Mention bot is now archived and the crates have change org ownership. I appreciate the time you've spent drafting the arguments but we're now an even smaller team and I don't think PRs are stalled for lack of opinions and reviews. The outstanding ones are blocked on available time to implement which hints that there is not more time available anyways.

If you have other ideas for improving the process, feel free to open a new issue 😄