At Stack Builders we use pull reviews for all code patches on our projects, and have been using this approach at least for the last few years. This has really worked out great, both to ensure that code meets certain quality standards, and also to make sure that someone else on the team is aware of the changes that are being made to the system. In that sense it's really a communication tool rather than just a tool for quality assurance.
We've noticed that some open source projects 1 2 use an interesting model of accepting Pull Requests from other committers. The reviewer will provide the usual feedback and when it looks good instead of merging will just say "LGTM" (either "looks good to me" or "looks good to merge" depending on your preference!) and then the commiter will merge the pull request himself/herself. I think this makes a lot of sense especially on teams where all contributors have commit access to the main repository - the developer could easily commit himself/herself but as a matter of convention will submit a pull request and let other developers get a notification about the changes and give them opportunity to provide feedback.
There are a few benefits to this approach:
-
the committer is always the best person to know when the PR is ready to be merged in, especially when there are some dependencies like configuration change on a server, PRs from other projects etc.
-
since the committer is doing the merging he's available right away to help with any issues, like CI failure, issues with staging deployment etc.
-
if a few PRs have been recently merged in and the PR was already "blessed", the committer can rebase one more time just before pushing the button.
-
sometimes the only thing that's blocking the PR is a small style change, typo etc. So someone can say "looks good but please fix these small changes" and the committer can fix those and push.
We've been using this new approach for a few weeks now and so far we haven't seen any issues. Even though it's a minor change in our workflow I think it makes it more asynchronous and enables us to get the code out a little bit faster. Have you tried out this approach? Let us know what you think in the comments!
