DEV Community

Discussion on: I'm Changing How I Review Code

Collapse
 
lmorchard profile image
Les Orchard

Oftentimes, I've been on projects where reviewing the PR means that you, as reviewer, are now also tapped for questions and follow up work on that bit of code. Later, when we look at git blame (terribly named) for further development, the reviewer listed is someone who may get tagged in.

So, you should run the code, because you might end up having to pick up the baton to fix or extend it next. Ideally, there is no single author of the code base. A PR is often a good point for spreading the responsibility and know-how. And spending some time as a reviewer to manually verify functionality can often catch things the bleary-eyed author missed. Even assumptions in automated tests can be wrong.

Another aspect: There's a such thing as too much engagement on a PR. Not everything needs the whole team dog-piling on it. Not everyone needs to feel required to robustly converse about every change. Most PRs, IMO, should be small enough for just one other set of eyes. (Not that I've always managed to adhere to that)