4 ms·

My code reviewers only find style/standards issues. I refuse to believe I'm that good... so does anyone have any tips on improving reviewer engagement?

morbidhawk 10y ago
I have the same exact problem, most of my teammates that review my code either obsess about variable names or code that wasn't done the same way they would have written it. It's like my code is being skimmed to bikeshed about it and not really read through to find behavioral, logic, or architectural issues. As I am the dev w/ the least amount of experience on the team I feel at a loss and I end up re-writing code all the time for issues that many times aren't actual logic errors. The most frustrating thing is a lot of the "different" ways I've written code is inspired by code I've been reading from some of the open source frameworks I use, and I've picked up on some patterns that might not be mainstream Clean Coder appropriate but effectively solve problems. I've realized that I personally value code that works properly a lot more than any style/standard issues. I've tried to be very thorough in my own code reviews, reading it in its entirety and pointing out areas that look like actual causes for concern in terms of how the code behaves in hopes they'll do the same for me.
guitarbill 10y ago
Automated linting? Then the reviewers can focus on actually reviewing? Other than that, for a while we tried the policy that if a commit you reviewed broke something, the reviewer had to fix it, not the committer. It doesn't really work though, as you might expect. Edit: I should point out that I'm convinced the main benefit of code review is that your team knows what code you've worked on and what you've changed. Catching mistakes is a nice bonus, but this kind of information transfer is a huge productivity boon.
xiphias 10y ago
Talk with them about it openly and respectfully. It's a people/communications issue, not a technical.

morbidhawk 10y ago

I have the same exact problem, most of my teammates that review my code either obsess about variable names or code that wasn't done the same way they would have written it. It's like my code is being skimmed to bikeshed about it and not really read through to find behavioral, logic, or architectural issues. As I am the dev w/ the least amount of experience on the team I feel at a loss and I end up re-writing code all the time for issues that many times aren't actual logic errors. The most frustrating thing is a lot of the "different" ways I've written code is inspired by code I've been reading from some of the open source frameworks I use, and I've picked up on some patterns that might not be mainstream Clean Coder appropriate but effectively solve problems. I've realized that I personally value code that works properly a lot more than any style/standard issues. I've tried to be very thorough in my own code reviews, reading it in its entirety and pointing out areas that look like actual causes for concern in terms of how the code behaves in hopes they'll do the same for me.

guitarbill 10y ago

Automated linting? Then the reviewers can focus on actually reviewing? Other than that, for a while we tried the policy that if a commit you reviewed broke something, the reviewer had to fix it, not the committer. It doesn't really work though, as you might expect. Edit: I should point out that I'm convinced the main benefit of code review is that your team knows what code you've worked on and what you've changed. Catching mistakes is a nice bonus, but this kind of information transfer is a huge productivity boon.

xiphias 10y ago

Talk with them about it openly and respectfully. It's a people/communications issue, not a technical.