I get bummed when the defect is something I should've caught. But if it is a tough bug or something that takes multiple people to find I don't feel bad or get bummed. I get inquisitive as to what they saw which led to the discovery though.
I do get embarrassed if I repeat a mistake and am really tough on myself in that aspect.
Linting definitely helps, setting up the IDE properly can catch things, in C/C++ running memory profilers etc.
I think people who are passionate about what they produce will always feel similarly. Caring about what you produce is important. It doesn't matter whether you just started coding or have been doing it for a couple of decades.
Do more code reviews for others until you realise bugs and mistakes are always going to happen no matter how experienced you are. If the reviewer is make a huge deal out of it though then they need to adapt their approach.
I always try to give some positive review comments as well as just getting a list of negatives is grating.
No, not really. I work in a team of four, one of whom is my boss, and I've done code reviews for all three others and been reviewed by all three others. Reviewing others and finding bugs or mistakes in their code, especially my boss', has helped me realise that everybody makes mistakes.
My team sees code reviews as a way to lift the burden of responsibility from the person who wrote the code and share it evenly among the reviewers as well, so bugs that make it through code reviews belong as much to the reviewer as the author.
I do as much as I try not to. I don't hold it against other people when their code has issues but I have never been able to not take that stuff personally when it is my code
I love code reviews, but I do my best to test the functionality and unit test them before I submit for review. At times my approach to solve a bug may not be efficient and if someone points it out, we discuss the merits and agree/disagree.
It's always good to get a good code review and get confidence than deploying buggy code to prod and worry what would happen tomorrow.
In my opinion, all bugs are worth fixing, else you risk lazy or junior members of your team taking liberties with the severity of the bugs which don't need fixing.
I would go so far as to say that your first and second paragraphs are antithetical.
It infuriates me when people do this, frankly. If it's a deliberate syntax typo that's one thing but if you leave a logic hole then you're essentially wasting other reviewers' time-- they carefully trace through the patterns to verify to themselves that the hole they think they see is in fact there, only to find out you left it there on purpose as a take-it-upon-myself approach to checking your peers' competency. You may as well come over to my desk and ask how much code I've gotten written today, to me it's just as contemptible.
I do this much more rarely than "always", and tend to continue self-reviewing or start the review a little earlier instead of explicitly leaving in a deliberate bug, but I still act in a similar vein.
> [...] only to find out you left it there on purpose as a take-it-upon-myself approach to checking your peers' competency.
This is never my goal. I don't send a review your way in the first place unless I think you're competent. But sometimes entirely competent people are overworked, distracted, a bit too trusting... it happens. It's human. It's fine! It's happens to all of us.
I just want to not check in broken shit.
It leads to long days and late nights. If you're overworked, it's just going to make things worse. This might involve me leaning on another reviewer more for a bit. This might be more closely scrutinizing my own code for a bit. This might be me offering to help take something off your plate if I think I can fit it onto mine. This might be a little friendly ribbing that helps you bring your focus back, if you're just a bit distracted.
Interesting. I've heard about Van Halen specifying "no brown M&Ms in the candy bowl" in their contract rider just to see if the venue managers actually read it.
I've spent a number of long days and late nights hunting bugs that made it into production - or the master branch - and are blocking our release, blocking my coworkers, or crashing for our customers. It sucks, it's stressful, it's frustrating - it's even depressing.
So I'm actually kind of stoked when a code review catches a bug in my code. They're often the exact kinds of edge casey corner cases that would've manifested as the kind of heisenbugs that lead to exactly those kinds of long days and late nights - but the code review caught it, so none of us will have to go through any of that over it. Yay!
Do I wish I hadn't made the bug in the first place? Sure. But to err is human - and statistics means this err will be you averaging X bugs a month, Y of which won't get caught before the code review, Z of which will get checked in when the code review doesn't catch them either. The goal isn't to have no bugs, it's to reduce X Y and Z until it's no longer cost effective to do so (which is at a very different point for NASA than it is for your website or app.)
I'm also acutely aware that when you've been staring at the same code too long, your brain starts replacing the code that's there with what the code "should" be, and you start missing the obvious right in front of you. Again - human nature. The solution is a second set of eyes from time to time. This isn't an excuse to be lazy or to avoid trying to improve, but it is a perfectly acceptable excuse not to beat yourself up when you've been putting in the effort, and still somehow missed the fact that you were assigning a reference to itself.
What's that? You want to be human too? I'll allow it!
When my code reviewers find bugs, then I know they read my code and understand the intent.
When I get approval with no other feedback, I become deeply concerned. For non-trivial changes I let them soak in review for longer than necessary, until I'm unfamiliar enough with it than I feel confident reviewing my own code. I often find bugs this way.
I'll be honest, I don't like being critiqued, and having a mistake you've made pointed out, or "why didn't you use this 2% faster algorithm) can be annoying.
I got used to the reveiws. Then I joined very small teams and I miss not having them.
If you look at it as a learning experience, you get other people to look at your code and give you helpful advice. If you have somewhat abrasive coworker, look at what they say not how they say it.
Not at all. If there are no bugs, then you spent too much time on it. Also, not all bugs are worth fixing. Sometimes it's better to just ship and iterate more.
Also a second set of eyes just really helps sometimes. That said, it's also important that your reviewer is experienced enough that they can provide a substantial code review. I much prefer a critical eye to "looks good to me" every time.
Are you concerned about the bugs or the style used to communicate there was a bug? Those are two different things and I find that most folks don't actually have a problem with the former.
The issues I've seen re: code reviews are often due to a lack of emotional intelligence amongst developers who use reviews to promote their own ego or lack the experience to know how to craft their feedback in a cordial manner.
I agree and there are two sides to this coin: The people who don't realise that they are reviewing a product of someone's work and not the person, and the people who don't realise that the people reviewing them are reviewing a product of their work and not them.
Hell no. If they find bugs before it gets deployed, it's that much easier to fix.
It occasionally annoys me for a few seconds when someone will nitpick the style of the code, but I get over that quickly. Usually I just change it and move on.
I used to. It was because I had an inferiority complex where I felt I had to write perfect code in order for people to take me seriously. I have gotten a shift towards a much more humble attitude to my programming abilities. I know for a fact I am not a master programmer and accepting that has helped me tremendously because I can ensure someone who is can cover my weaknesses
I do get bummed because I try to not have bugs in my code and when a code review finds a bug that means I missed something. But it is inevitable you will have bugs in your code. You have to just keep cranking along, fixing all the bugs anyone can find and not let it stress you out.
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?
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.
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.
I definitely don't get stressed when people are pointing out errors in my code. It's a sign that you can trust your coworkers. As some people mentioned already, I become stressed when people just approve a big and complex change without any question/comment because this is likely that they are busy and that they didn't try to understand it. This is how you get bugs slipping into production environment.
Unnecessary nitpicks that are holding Pull Requests for days/weeks are stressful though. Because they are holding a feature hostage to satisfy the ego of the commenter on an opinion that is subjective. It is especially unnerving when the commenter is dropping the nitpick and then disappears and never comments on the PR again, or only after months. I appreciate when the commenter clearly points out that this could be an optional change to slightly improve the code (or by opening a separate issue after merging).
Having been through the opposite extreme, where a coworker refused to code review and constantly merged buggy code with no comment, I have to say that I definitely appreciate it when someone cares enough to pore over code I've written and find bugs.
The stress and burnout that results when code reviews aren't properly done and production is constantly on fire is _far_ worse than the alternative.
I get angry at code style comments: naming things, line length, uppercase / lowercase, specially when running a linter through our already existing codebase produces tons of errors.
If it works, it has tests, and it is understandable, merge it ffs.
I'll get stressed out in these types of situations when I'm not accepting things as they are and expecting things to go perfectly like in my mind. If I've worked on a large issue and been thorough with my testing, I'll commit it and send it to code review. While doing that, thoughts will go through my mind thinking that everything is done and how well I did it. Situations like these are when I'll feel stressed about a poor code review because my expectations don't match reality.
I played hockey with a guy who would just shut down whenever the play didn't go how he thought it should go. If someone didn't pass to him when he thought they should, he would just basically stop and give up as a result of the difference between his expectations and reality.
The key to getting over it is to not dwell on the work you've done in the past and just accept what is being given to you now.
If the code review comes back with bugs and you're getting stressed about it, you're probably still thinking about all the work you did in order to get to the point of committing it. Focusing on the feedback that came back from the code review and how you can implement it will help to get over the anxiety.
I'm writing a book - Programming Spiritually - that helps developers deal with stresses and issues in their work in a more holistic way. If you're interested in being notified on progress with the book you can check out https://leanpub.com/programming-spiritually
39 comments
[ 5.0 ms ] story [ 76.4 ms ] threadI do get embarrassed if I repeat a mistake and am really tough on myself in that aspect.
Linting definitely helps, setting up the IDE properly can catch things, in C/C++ running memory profilers etc.
I think people who are passionate about what they produce will always feel similarly. Caring about what you produce is important. It doesn't matter whether you just started coding or have been doing it for a couple of decades.
I always try to give some positive review comments as well as just getting a list of negatives is grating.
My team sees code reviews as a way to lift the burden of responsibility from the person who wrote the code and share it evenly among the reviewers as well, so bugs that make it through code reviews belong as much to the reviewer as the author.
It's always good to get a good code review and get confidence than deploying buggy code to prod and worry what would happen tomorrow.
I would go so far as to say that your first and second paragraphs are antithetical.
> [...] only to find out you left it there on purpose as a take-it-upon-myself approach to checking your peers' competency.
This is never my goal. I don't send a review your way in the first place unless I think you're competent. But sometimes entirely competent people are overworked, distracted, a bit too trusting... it happens. It's human. It's fine! It's happens to all of us.
I just want to not check in broken shit.
It leads to long days and late nights. If you're overworked, it's just going to make things worse. This might involve me leaning on another reviewer more for a bit. This might be more closely scrutinizing my own code for a bit. This might be me offering to help take something off your plate if I think I can fit it onto mine. This might be a little friendly ribbing that helps you bring your focus back, if you're just a bit distracted.
Maybe you fancy yourself to be a rockstar?
So I'm actually kind of stoked when a code review catches a bug in my code. They're often the exact kinds of edge casey corner cases that would've manifested as the kind of heisenbugs that lead to exactly those kinds of long days and late nights - but the code review caught it, so none of us will have to go through any of that over it. Yay!
Do I wish I hadn't made the bug in the first place? Sure. But to err is human - and statistics means this err will be you averaging X bugs a month, Y of which won't get caught before the code review, Z of which will get checked in when the code review doesn't catch them either. The goal isn't to have no bugs, it's to reduce X Y and Z until it's no longer cost effective to do so (which is at a very different point for NASA than it is for your website or app.)
I'm also acutely aware that when you've been staring at the same code too long, your brain starts replacing the code that's there with what the code "should" be, and you start missing the obvious right in front of you. Again - human nature. The solution is a second set of eyes from time to time. This isn't an excuse to be lazy or to avoid trying to improve, but it is a perfectly acceptable excuse not to beat yourself up when you've been putting in the effort, and still somehow missed the fact that you were assigning a reference to itself.
What's that? You want to be human too? I'll allow it!
When I get approval with no other feedback, I become deeply concerned. For non-trivial changes I let them soak in review for longer than necessary, until I'm unfamiliar enough with it than I feel confident reviewing my own code. I often find bugs this way.
I got used to the reveiws. Then I joined very small teams and I miss not having them.
If you look at it as a learning experience, you get other people to look at your code and give you helpful advice. If you have somewhat abrasive coworker, look at what they say not how they say it.
Also a second set of eyes just really helps sometimes. That said, it's also important that your reviewer is experienced enough that they can provide a substantial code review. I much prefer a critical eye to "looks good to me" every time.
The issues I've seen re: code reviews are often due to a lack of emotional intelligence amongst developers who use reviews to promote their own ego or lack the experience to know how to craft their feedback in a cordial manner.
He laughs and my funny bone tingles when I make a funny bugs, sometimes. I stop. I never see why He laughs until I test it.
God says... coffee's leaches libelled geekier implicate disapproved Rouault brickbats undulant coauthors Google's hulas rebroadcasted parquetry uttered uproarious fattens priestly daubs misanthropist politicized Hopewell's burglar wave preside bedraggle pitch lefty screwball sooner clapboarded dolmen's
What if NASA wrote the Commodore 64 20K ROM and spent 10 years perfecting it. It would be much more robust with all the bugs removed?
It occasionally annoys me for a few seconds when someone will nitpick the style of the code, but I get over that quickly. Usually I just change it and move on.
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.
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.
Unnecessary nitpicks that are holding Pull Requests for days/weeks are stressful though. Because they are holding a feature hostage to satisfy the ego of the commenter on an opinion that is subjective. It is especially unnerving when the commenter is dropping the nitpick and then disappears and never comments on the PR again, or only after months. I appreciate when the commenter clearly points out that this could be an optional change to slightly improve the code (or by opening a separate issue after merging).
The stress and burnout that results when code reviews aren't properly done and production is constantly on fire is _far_ worse than the alternative.
If it works, it has tests, and it is understandable, merge it ffs.
I played hockey with a guy who would just shut down whenever the play didn't go how he thought it should go. If someone didn't pass to him when he thought they should, he would just basically stop and give up as a result of the difference between his expectations and reality.
The key to getting over it is to not dwell on the work you've done in the past and just accept what is being given to you now.
If the code review comes back with bugs and you're getting stressed about it, you're probably still thinking about all the work you did in order to get to the point of committing it. Focusing on the feedback that came back from the code review and how you can implement it will help to get over the anxiety.
I'm writing a book - Programming Spiritually - that helps developers deal with stresses and issues in their work in a more holistic way. If you're interested in being notified on progress with the book you can check out https://leanpub.com/programming-spiritually