How to encourage actual participation in code reviews/pull requests

Question

I'm currently acting as team lead for a small group of developers, all of which (including myself) are fairly new at the company. Part of my job is setting up the development workflow, and in that vein I've made it mandatory that every feature goes through a pull request and code review before being merge to the main development branch (we utilize gitflow).

The problem I'm seeing is that one of my team members is consistently approving all pull requests, regardless of size within ~5 minutes of them opening, never having any suggestions or comments. I can only assume this means that they're not actually reading the pull requests and just rubber-stamping everything that comes on their plate for review (either that, or I'm a flawless programmer, which seems unlikely).

Confronting them seems like it could cause some unnecessary conflict, as I can't prove that they're not reviewing the PRs, and I don't want to create a situation where people feel they're required to leave feedback.

How can I encourage active participation in the code review process and avoid rubber stamping?

Answer 1

Tl;dr - you are all new to the company, so say that you want to achieve uniformity of reviews & review some of your code, as a group.

---

Reminiscing; I once had to introduce software engineering to a company that had only previously done one & two person projects. Then they landed "the big one".

I introduced everything. Explained why we needed documentation & what kind, created document templates & document numbering system. Explained requirements & requirements tracing; set up some kind of a coding standard; got everyone to standardize on the same development tools; why we needed testing; why it ought to be automated; how to test; yadda yadda yadda; explained reviewing.

And I ate my own dogfood all the way.

The first design doc, test spec and first piece of code that were reviewed were mine. I had some trouble, but got them to understand that they were not reviewing the person. We were all going to be reviewed, and there were going to be improvements suggested.

For writing a design document, I told them that I was just the secretary. We had had a series of meetings; I had taken notes, and turned "our design" into a document. Did we miss anything? The document was only "mine" briefly; after the review the document belonged to all reviewers, and all were responsible for any problems with it.

For the code review, I had been “the coder” only inasmuch as I had turned “our” design document into code. Did I make any mistakes in transcription? Could I have done it more elegantly, handled errors better, etc? I won’t take comments personally, as we are all trying to make a product together. But, code review, like all reviews, is a responsibility, and after this review, it is no longer “my” code, but “our” code, and when the test team find the inevitable problems, we all share the responsibility.

This all took place in Asia, where the culture does not invite criticism, and certainly not of those above on in the pecking order. Which is why we tore my stuff apart. I and my (western) partner both pointed out a few things, to break the ice, and my explanation that the group owned the review item, not an individual, got them on board. This ought not to be necessary in the West, but often is.

It sounds like your guys don’t need quite that level of explanation, but you might be able to tease something from it.

And you can always just read my Tl;dr ;-)

Answer 2

People should usually feel required to provide meaningfull feedback, not just any feedback. I haven't seen any code ever were you could not give such feedback, if you put a bit of thought into it. This feedback is the reason to have reviews in the first place.

Well, one step could be to take a deeper look at the issue. Why does the dev act that way? Is he overworked and just has no time to thoroughly review any code? Is he afraid of saying something negative about your code? Does he have the neccessary skills to review your code? Try to find out what keeps him from providing feedback and try to fix the issue.

Lastly: Not everyone is able to do good reviews at all. Some people are too conflict-avers to provide meaningfull feedback. Some just aren't able to see the 'big picture'. Some despise the process of reviewing itself, because they prefer to do 'actual development'. Maybe you want to have dedicated reviewers for this process.

Answer 3

1. Do they know enough to provide feedback? I am technically a code reviewer at my company. I am also relatively new to our stack. So I don't have the expertise to spot problems, but there I am code reviewing anyway.

2. Does providing feedback make sense? Good software engineering practices fall by the wayside as the sprint deadline comes up except in very engineering-centric organizations. Management often prefers the hack (perhaps not explicitly, but in what they reward and express concern about) as it means you ship on time.

3. A lot of devs are not confrontational people. I will only confront someone on the most solid grounds if there are any stakes to it. In this case, it would be my relationship with my colleagues. I might have done finance were I more comfortable with it. In case of doubt, I move on.

4. Different devs have different motivations. A colleague of mine loves the journey, which makes perfecting the engineering interesting to him. I admittedly enjoy the shipping part moreso than the engineering part, especially the engineering of other people's code. I will do as letting others put in problematic code will kill shipping, but it is work, not the fun part of development. I know people who will ship anything that works. I know others who are pedantic about squeezing out every last bug and scrap of efficiency before shipping. The latter will love code review. The former sees it as a barrier to the fun part.

Answer 4

How can I encourage active participation in the code review process and avoid rubber stamping?

Don't just send code review to people but actuel perform a pair review with people.

Set-up a short meeting with you reviewer(s) where you'll explain your work. Not just your code, but your work. Maybe your code works but it doesn't solve the problem stated or you missed edge cases...

Confirm with reviewers their understanding of your work and actively ask if they have questions or remarks on it.

Sometimes it'll be quick, sometimes it can take a lot more time. But the longer the work, the bigger the need for a well done code review.

Answer 5

The problem I'm seeing is that one of my team members is consistently approving all pull requests, regardless of size within ~5 minutes of them opening, never having any suggestions or comments. I can only assume this means that they're not actually reading the pull requests and just rubber-stamping everything that comes on their plate for review (either that, or I'm a flawless programmer, which seems unlikely).

The end goal of any code review is to catch bugs and ensure code standards. The question, ultimately, is if broken products are pushed out or if code standards aren't being met. If that is the situation happening, then it would mean you need to step in and enforece code review and making sure people read into it.