In part 2 of my series on dev team interactions, I’d like to talk about conducting good code reviews. Most dev teams will find themselves in a situation where code reviews are necessary, and in my experience many do them very poorly. I’ve even worked in companies that had such a negative code review culture that people left the review sessions upset, even considering quitting. With a few easy adjustments, you can quickly learn to conduct excellent and positive code reviews with your team.
The Ground Rules
A code review is a process. Like any good process, clear rules need to be established and followed to ensure a consistent experience. Here are mine:
- Attack the code, never the person. Criticizing code is OK, but people are not code. It’s never OK to criticize the person and make them feel bad. Focus strictly on the code output and never make it personal.
- Don’t laugh or make negative jokes. A person who has their work on display – often on a projector in front of others – is feeling self-conscious as it is. Don’t snicker at their work. Avoid joking about their decisions. I assure you they are trying to do the right thing.
- Set a strict time limit as a means of focus. Make the code review 15 minutes, 30 minutes, or even 1 hour. Stick to this schedule. This forces you to prioritize the important stuff and ensures (intentionally) that you can’t review absolutely everything the person has done. Don’t take unlimited time to scrutinize every single line of code written. Ever had someone comb through your code line by line, making commentary as they go? “And how did that make you feel?”
- Thank the person for sharing their code with you. A code review is an intimidating, scary thing – especially for developers on the junior side of the spectrum. Set the tone correctly by being appreciative of their time and sharing. Make it known that they are valued and you appreciate their work up front. This will help them feel relaxed and learn to enjoy code reviews, which in turn will cause them to want to share more of their code willingly in the future as well.
The above rules serve only as an example from my personal experience. You should create rules that work for your company and work culture. Use real and practical standards that matter to your team, not just theoretical ideologies that someone on your team read in a book. Always remember: a process is only as good as the people that follow it, so try to be consistent with whatever rules you decide on. And if they aren’t working well in practice, change them up!
Seek Understanding, Not Power
The general goal of your code review attendees should be to seek understanding, not explanations. Avoid an us-them conflict or standoff. This can be easily accomplished with a subtle shift in communication approach. Rather then asking aggressive questions that demand the reviewee explain the code to your team such as “why would you do it that way?” or “how could that possibly work?” you need to ask questions to promote understanding the code instead. Once your team and the reviewee correctly understand the code being reviewed, you can proceed to discuss a different approach without conflict or hurt feelings.
When you see some code that makes you think “what the hell were you smoking?” (explain) you should instead ask “Can you tell me why you wrote the method this way?” (understand). These statements are similar but the former gives the power to you while the latter empowers the code reviewee to share their knowledge and thought process in a non-defensive way. This approach to questioning takes a bit of practice, but is very powerful. Some other examples of how you’d change common code review aggressive thoughts and statements from explaining to understanding:
- “This class name is wrong” becomes “This class name doesn’t conform to our standards, was that intentional?” You’ll probably find that they say it wasn’t, and agree to fix it on their own volition.
- “I see a bug in your code!” becomes “I think there’s a null reference exception on line 28 for variable X, do you see how it happens?”
- “This is terrible” becomes nothing – keep your mouth shut. There’s no value in such a statement other than to make the person feel bad. If the code truly is terrible, express these concerns to the person (and maybe their manager) privately for further investigation and resolution.
People are not robots, and will never conform perfectly to your shop’s coding standards. That’s OK. Pick your battles, and call out only the major violations. Let the little things (like naming and spacing) slide. If the dev is hitting 90% of the standards, the rest of the team can pick up on the 10% deviation without much worry or effort. That is to say, the code won’t be that different than what they expect to see.
If you seek understanding consistently, you’ll find that the person drops their defenses and ego, and instead feels encouraged by your positive approach and attitude. They’ll even start pointing out and suggesting fixes to their code themselves – right on the spot – which is the golden sign of trust and confidence. The best kind of code reviews are the ones that a person points out issues themselves to fix, rather than the team having to do it for them.
Avoid Opinion Wars
When reviewing code, there will be two general categories of issue:
- Objective, fact-based issues
- Subjective, opinion-based issues
Focus explicitly on objective issues, and disregard all subjective issues. An objective issue is an exception or oversight in conforming to well-defined coding standards. A subjective issue is you not liking the way the person solved the problem, or feeling that what they did is not what you would have done. You’re right because you’re different people. Striving to make others conform to your thought process as the one true standard is egotistical, destructive, and stressful. In the long run you will be unhappy because there will always be a gap between others and yourself. This is natural so go with the flow. Allow members of your team to be individuals and write code with their own flair and flavor. It’s OK, it will be OK, and you will be OK. I promise.
Code is for computers, but programmers are humans. Be kind to each other and always remember that the thing on the other end of the code review is a real live person with feelings and emotions. Treat them with respect in what you say and do. Recognize that they are valuable and thank them for their hard work. Together you will create a great team that others will love being a part of!
Well, I’ve utterly failed to blog at regular intervals, writing only three posts in 2016. Ouch. To be fair, one of those posts is insanely famous (the one about NPM and left-pad.js), but still, I’ve really let my readers – and myself – down.
So, I resolve to write a blog post every single week of 2017, starting today. This will probably mean that I write slightly shorter posts, and maybe even multi-part series posts. My traditional style has been “come upon something that is really bothering me or is really tricky, and proceed to blog about it in great detail writing thousands of words for all to benefit from” which doesn’t really scale well. Instead I plan to take the approach of “write about a new or interesting topic each week, and see what people like and what they don’t like” which will hopefully be better.
This week’s post is about compassion, especially in the field of programming. Let’s use an example that is both recent and practical, if a tad emotional.
3 days ago our dog Ruby died. She was 16 and lived a good, long life. Having said that, her death was unexpected and rather painful to go through. My wife and I were up with Ruby all night comforting her as she slowly passed away from sudden heart failure. The last breath she exhaled will forever be seared into my brain. It feels like it’s all I can think about right now.
My wife and I have been grieving for the past 72 hours in our own ways. This has surfaced as mostly a mix of depression, tears, quick but sad chats about the things we miss about Ruby, and sorrow every time we look somewhere in the house for her only to find that she is no longer there. It sucks.
What’s important to realize is that I am not unique or special in this circumstance. Animals die all of the time, and you don’t feel sad about them because you never met them. They are out of sight, out of mind. Yet they are out there, and the passing of good dogs and cats – as well as other households pets – is happening every single day. Even as you read this.
These events occur in the scope of individual’s lives. There is no national news story about the passing of Ruby to make us all aware of it and help us all feel the effects of it. There never will be. This event is confined to the lives of me, my wife, and our extended family. We suffer through it silently (other than my writing about it and a few social media posts).
This silent suffering is the point that I want to drive home. Others out there are silently suffering too. It might be the death of a family pet, or it might be something else entirely. It’s not a contest of who has it the worst. I want you to simply realize that crappy things are happening to people, even if they are not in your domain and thus you are not aware.
Odds are that someone in your life or workplace is suffering with something that troubles them. Right now for me it’s the passing of my dog. For a close friend of mine it’s stress about work and finding the next permanent job. For another person I know it’s about being a brown man in America and how scared they feel about the future. We all battle demons every single day.
With this in mind, I ask you to be compassionate everywhere, but especially at work. You work with people, not cogs of some machine (even if management may think of them in that way). These people have real feelings and some of them are surely fighting battles you’ve never even fathomed.
Your responsibility as a good member of the IT field – and as a good citizen of this planet – is to demonstrate compassion. Be nice to everyone you work with until you are given a reason not to be. Consider that they might be having a bad day if you feel they’re lashing out or being an asshole, and forgive them. Try to sympathize with what frustrates them. Work to understand them beyond their role at the company and their job output.
We are all in this together. Now more than every we need to be compassionate, respectful, kind, and considerate to others. It is my opinion that our industry has a long history of being hostile to certain people and demographics – such as women – and we must each work individually to change that.
My last request to you: please think hard about what you’ve read here as you begin your first week of work in 2017. I know I will.
Picture this situation: you woke up this morning to find that there’s no water coming through your valves and taps. No sink water. No shower water. Having no plumbing experience, you call around for a plumber.
Plumber #1, let’s call him Mario, tells you he can’t be bothered to come check out your issue because it’s minor and he’s very important and too busy for it. You explain that you really need a plumber, and he explains he’ll do it for 1.5x what everybody else costs, and only if you have lunch and coffee ready for him when he arrives. You have no water, keep in mind, so making coffee is an extra special effort.
Plumber #2, let’s call him Luigi, agrees to show up and assess the damage. He walks in your door, doesn’t bother to shake your hand or introduce himself, and immediately gets to work. A few minutes of inspection later and Luigi is telling you that all of your plumbing is wrong. None of the pipes are plumbed how he’d plumb them, so they’re wrong. He needs to rip out the whole plumbing system and do it all again from scratch, which will cost you thousands of dollars. He has a reputation for being skilled: his past references love him, but this is going to cost a lot of money and take a lot of time. It’ll be at least a week before you have running water again.
Plumber #3, let’s call her Peach, asks you some detailed questions about the problem over the phone. “How long have you lived in the home?” “Have you had any prior plumbing issues?” “Has this happened before?” “Do you have any other sources of water?” Based on this analysis, she tells you she has a good idea of what the issue is and agrees to come over later that day, since she’s in the area anyway. She shows up, immediately discovers that the pesky neighbour kids shut off your water main, turns it back on, and then gives you some advice: “it’s not something you need to do right now, but you should upgrade to a locked main switch that operates with a key so that this won’t happen again! Call me if you want something like that installed.”
Now, which one would you hire?
Back to Programming
Let’s take this [terrible] analogy back to programming. Plumber #1 is a rock star developer who demands special treatment and pampering. Plumber #2 has a huge ego, a serious case of not-invented-here syndrome (they didn’t plumb the house so it’s wrong!), and might also be a rock star. Plumber #3 is pragmatic, friendly, sensible, analytical, and balances cost of time and money with urgency. For me personally, #3 is the only one I’d happily hire.
It seems so obvious that egos and not-invented-here cost a lot of money and do a lot of damage in that analogy. It’s less evident in the workplace. Some companies foster the ego and demand to hire rock stars, believing that the “ten ex” developer will outperform others they could hire. What they don’t realize is that the cost of this hire almost certainly outweighs the benefits.
There’s no place for ego in programming. An egotistical programmer does incredible amounts of damage to those around them. They do cultural damage to your company, emotional damage to their peers, and financial damage to your budgets. Those rewrites aren’t cheap!
Don’t be a rock star. Rock stars are cold, hard to approach, notoriously unfriendly and self-absorbed, make no time for others and teaching them (because everything’s about ME!), and are frequently dramatic and rude (to get in the “news” – after all, any publicity is good publicity).
Seriously, "rock stars" are arrogant narcissists. Plumbers keep us all from getting cholera. Build functional infrastructure. Be a plumber.
— Molly Sauter (@OddLetters) June 17, 2016
I don’t know about you, but I want to work with Peach. I prefer a humble, thoughtful, talented peer that can teach me while maintaining respect and a relationship of trust. That’s the kind of workplace that I get up every morning excited to go to. Peach is probably not perfect, and has her flaws, but the lack of an ego and attitude means she’s willing to learn and grow – and that’s the key to success in this industry.
If this analogy hit a little close to home, it’s time for a little introspection. The beauty of the human condition is that we are always on a path, growing and changing. Who you are today is not who you are forever. Think about which plumber you resemble above, and then adjust. Next time you’re about to say “that’s all wrong!” or “we have to write it ourselves!”, pause and think: there’s probably a decent a reason that the code exists in its current state. Is now really the time to undertake a very expensive rewrite / greenfield project? Probably not.