The process on our team is to create a feature branch off of develop, do your work and then push it up to the repo. Then when it’s all done, you create a PR and have someone from the team be the “primary reviewer.” This person’s job is to review the PR, make sure that you’re doing the right things in the right way, then they have to pull down the branch and test it before they can approve it.
We got to this point after many iterations about how we wanted to do PRs. At first we opened up all the PRs to everyone on the team and after a majority of the people approved it, we would merge it in. Then we found that a lot of people didn’t want to review and the PR process would drag on. Having a PR open for a long time causes its own problems, as by the time the code is ready to get merged, it’s old and often has a lot more conflicts.
Next we tried to only have a couple people PR the code, the “leads” if you will. However, that didn’t work out very well, as people would often forget to address the comments etc.
We tried PR buddies, and finally came to this incantation. Despite all the struggles of PRs, everyone on the team seemed to agree that they served an important purpose, and wanted to continue them.
Recently, however, I had a revelation about PRs. I always approached PRs as “Let me review this code to make sure it meets our standards, and is clean and working.” That would be my motivation, and sometimes I felt like “I just don’t have time to do this PR because I have my own stuff to do.” But then, within the last couple weeks, something dawned on me. A PR is not just a way for me to make sure other people on the team are doing things the right way (and for them to make sure I’m doing them the right way) it’s also an opportunity for me to learn.
When I open up someone else’s PR, I get a chance to see how they tackled the problem, or how they solved something. Sometimes that’s as simple as seeing them use some new lodash function that I’ve never seen. Other times, it’s seeing a different paradigm or approach to the code. Still, other times, it’s seeing something like a new way to test the code.
I’m a big proponent of writing clean code because I believe that we’ll read our code many more times than we’ll write it. But it never occurred to me that the first time I’ll read that code is in a PR. I’d like to say that since I had that revelation, every PR has been walking in the light, but it hasn’t. I still find myself, at times, looking more to make sure that the code meets the team standards, and forgetting completely that I have a chance to learn.
If your team does PRs, I’d highly recommend trying to take the approach of learning something from a PR, instead of simply judging it.