[pwmarcz]

We had a similar problem - a lot of pull requests being stuck in a back-and-forth remarks and fixes. The "social" solution was to increase the amount of communication in the team: instead of everyone having their own tasks and working in a silo, we made multiple people (ideally, the whole team) responsible for a task, splitting work as we go. The technical solution was to use pair programming instead, at least for some changes. Both worked really well.

Pair programming achieves a similar result to code reviews (another person knows the code very well, and is able to criticize it and improve on the design). It also does it much better because the review is done as the change is being made, and not after the fact, which improves the understanding of the reviewer and allows them to give immediate feedback.

The old code review process still works as a fallback, and is especially good for small but tricky changes: I need to do something novel, it's not a big change but there are a lot of ways to do it, can you comment on my approach.

One other thing that we also ended up doing is "ping-pong pull requests": if you give me a pull request, I don't just criticize it leaving the issues for you to fix, I fix them and leave that for you to review. Then you can do the same, etc. As a result, both people get better acquainted with the code, instead of just one of them being the "owner" of a change.

[satyajitranjeev]

We tried a few methods and eventually went to pair programming. But never tried the ping-pong pull requests. It sounds interesting. What gets me off for pair programming is that it's not always feasible. If the other person who could work with you is on another task you spend a lot of time try to play catch up. How well did the ping pong work?

[pwmarcz]

It works well. I think with the original pull-request process we basically had a mental block that only one person is allowed to write the code, and the reviewer can only comment. This can be frustrating especially if you know exactly what needs to be fixed. If you just go ahead and make the change, you save both people's time.

I guess one place where it fails is that it's not very educational. If you're tutoring a less experienced programmer, you don't want to fix the mess after them, you want to teach them how to doing without you the next time around. In that case, I either leave the fix to the author, or fix the issue as they watch.

[skewart]

I'm really curious how the ping pong PRs worked out. I've often wanted to do this - both when reviewing code and having code reviewed. Especially for relatively small changes it can be almost as much work to describe a change as it is to implement it and give a little explanation for why.

[pwmarcz]

See the other comment in this thread.