Code review review is the manager’s job

By kiyanwang - a day ago

Showing first level comment(s)

The author tried to be clever with the title, but failed to take into account how human perception works. The second "review" gets filtered out like that gorilla in the basketball video.

ndh2 - 18 hours ago

The manager can do code review if they're a player-manager. If they're a people manager, it's a lot harder to meaningfully review. Managers usually hired in to manage after the team grows (and splits), rather than trying to convert developers into managers, so they may not have a good idea about the code at all.

I'm more inclined to think that it's senior technical responsibility to review, since they'll have a lot more knowledge of the codebase, its evolution and future direction, potential gotchas and various cultural dos and don'ts.

For sure, it's important to keep an eye on the code review culture, though. It's easy for code review to degenerate into nitpicking small things like whitespace, naming, coding conventions - these should be handled by tools instead. And of course if people are being assholes, they need to be pulled up on it.

barrkel - a day ago

Yeah, I see more projects with bad code review culture than good. There are many things that managers can do about it.

* Assign enough time for code reviews - otherwise developers will just care about their own code submissions and let reviews rot.

* Establish productive code review rules. E.g. always go from big picture to details, otherwise you'll waste time on details that you end up throwing away.

* Make reviewers co-responsible for the quality of code reviewed, otherwise instant acceptance for everything becomes attractive.

ahartmetz - a day ago

Before jumping in with your thoughts on how managers should or should not be involved in code review, note that this is about code review review. Reviewing the code review process, and reviewing the code reviews.

EliRivers - a day ago

I agree with this in principle. Gets a lot tougher in practice, though.

"Manager" really shouldn't be a job in tech, at least when it comes to individual teams. Management should be a skill, just like all the other dozens of skills tech teams manage in various ad-hoc ways.

Groups of teams plays out differently, since you're really starting to work at the meta level. Even then, though, management is just one of a bunch of skills necessary. (At that point, however, it may be so much of a primary skill to warrant a separate role)

DanielBMarkham - a day ago

How do these benefits compare to what you get from pair programming? If the pair works well together I think pairing provides more learning and better quality.

maxxxxx - a day ago

Are there any field studies that demonstrate the impact and efficiency of these practices? Is there a body of evidence to support the assertions that are made in this post?

sgt101 - a day ago

But is the code review review review the manager’s manager’s job?

dave_sid - 17 hours ago

This is a really interesting read. Thanks. I have found in my own experience that developers tend to include the same people over and over again in their PRs and what ends up happening is people tend to just "trust" what the submitter has committed. This often leads to PRs that are almost blindly approved. By involving management to keep the PRs as a tool for discussion (social and technical), I think this could be avoided. It's probably also a good idea to mix up the reviewers (a healthy mix of people familiar with the code and those not familiar with it).

bradenb - a day ago

-1 Moving the evolution of a groups code into the hands of the person who works with it the least is an anti-pattern. Driving change from within a team can be facilitated by a manager who reads the group's code, but any manager who thinks they are going to be the judge of what the team is achieving is deluded and overly enthusiastic about their authority.

atomicbeanie - 19 hours ago