Timeline for How to code review without offending other developers
Current License: CC BY-SA 3.0
15 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Oct 10, 2013 at 19:10 | comment | added | Joel Etherton | @0A0D: Nitpicking is a fairly relative concept. My response to anyone who had this complaint to one of my code reviews would get this response: "Do it right the first time, and I won't have to nitpick." If it's brought up in a code review then it should be for a reason, and not just because "I don't like it." The reason might be a very small nitpick, but if it isn't right, it isn't right. | |
| Oct 10, 2013 at 19:07 | comment | added | Engineer2021 | I don't mind code reviews at all, but don't nitpick. That means I have to address the perceived "defect" x number of times. That wastes time and is frustrating. | |
| Oct 9, 2013 at 2:56 | comment | added | Bryan Oakley | @JoelEtherton: ok, no problem. I couldn't tell if you were merely speaking an opinion or if you had read a study or book or something. | |
| Oct 9, 2013 at 2:08 | comment | added | Joel Etherton | @BryanOakley: I don't cite references for statements that are clearly opinions based on personal experience. | |
| Oct 8, 2013 at 22:19 | comment | added | Bryan Oakley | You wrote "This seems to be a pretty common prevailing attitude among some developers.". Do you have any references to back that up? I've never seen a developer with this issue. Maybe it's not as prevalent as you think. Admittedly, I seek out jobs where I work with like-minded people in relatively small companies, so my personal experiences may be vastly different than yours. | |
| Oct 8, 2013 at 15:22 | history | edited | Joel Etherton | CC BY-SA 3.0 |
added 3 characters in body
|
| Oct 8, 2013 at 14:12 | comment | added | Joel Etherton | @JuliaHayward - All good points, and the code review is the beginning of that dialog. If a reviewer says, "You didn't follow standard X", and there can be a case made that "Standard X" no longer has any true application then that should be the beginning of the questioning process. | |
| Oct 8, 2013 at 13:13 | comment | added | Julia Hayward | As an addendum to (2), the code standards and other design documents should be treated as living documents that are regularly reviewed. They should be considered open to challenge by anyone as long as the challenger can back up his criticism. There's always a danger that practices (which were desirable or essential years ago) fossilise, and you do not want your team to follow blindly. | |
| Oct 8, 2013 at 13:07 | history | migrated | from workplace.stackexchange.com (revisions) | ||
| Oct 7, 2013 at 20:37 | vote | accept | ConditionRacer | ||
| Oct 7, 2013 at 19:21 | comment | added | Affe | As someone who spends a lot of professional time code-reviewing, #3 x 1,000. Always explain why, even if it is the most obvious thing in the world to the most junior team member, always give a rationale for a suggested change. Eliminates the majority of problems. | |
| Oct 7, 2013 at 15:54 | comment | added | HLGEM | 'The code doesn't progress until the reviewer approves it, and the reviewer won't approve it until the questions are answered." I thnk this is key. If you do code review after teh code has moved to Qa or even prod, then people will resist chaning it even more. If they know that they have to pass code review, they will be more open to making changes. | |
| Oct 7, 2013 at 15:51 | comment | added | Joel Etherton | @AdamV: I've found this less necessary in recent weeks. I originally used it as a tactic to instill in the other developers that I welcomed their feedback and challenging. A slight subterfuge, but it has paid off dividends I think. | |
| Oct 7, 2013 at 15:44 | comment | added | Adam V | Excellent answer. I especially like the sentiment of "check in code that wouldn't pass my own review"; it may not even be necessary to do that, so long as people understand that you're exhibiting the behavior you want others to emulate - seeking out comments on your code, taking comments under advisement without taking them personally, etc. | |
| Oct 7, 2013 at 15:35 | history | answered | Joel Etherton | CC BY-SA 3.0 |