DEV Community

Discussion on: How to deal with an argumentative code review?

Collapse
 
amt8u profile image
amt8u

Too late but still gonna add my experience.

Sometimes its not about the attitude but rather about the environment a person is in. In one of my previous organization, code reviews were done only in hierarchy i.e. Sr Dev will review Junior Dev's code, Lead will review Sr. Dev's and so on. And somehow the review comments were taken in a negative sense. As if receiving a review comment meant that you didn't do proper coding in the first place and indirectly could affect your appraisals.

When I joined another org after that, I was surprised to see a 18 year experienced person asking me to review his code. I could certainly not find out errors or misses. But to start with he asked me to write a comment even if I have a question. Its really easy to discuss when you have the context in hand. It also helps you

Also the feedback and its related changes also take time and effort. You need to account for that. In one of the instances I got some feedback and I let the team know that the feature cannot be completed because the changes required for it will take some time. It was understood by all and that feature was move to next sprint. If at that instant I was made responsible for not delivering on time, I could have become a hater too:-P

One last thing is about the communication medium. Sometimes its really difficult to make out the tone of the statement. Many people get offended if they feel the statement is rude or impolite. Maybe in person you can convey your expression easily but in writing its a challenge.

In the end its up to the culture you are in. You can always move out and find a position where you fit in.

Collapse
 
190245 profile image
Dave

@z00md , firstly, thank you for a thoughtful response, and apologies that I missed it all that time ago.

Things in my situation have changed quite dramatically. For starters, the problematic person I was posting about has been made redundant (along with others, unrelated to my issues around code reviews). But also, I've moved from Senior Developer to Architect / Lead Developer / Dev Manager (as a consequence of the company restructuring).

The very first thing I set in stone is that anyone can read anyone's code. There's no need for someone more senior to approve it, so long as someone follows due diligence, I'm happy. I still write code when I can, and I deliberately assign the reviews to Junior Developers. Hopefully they'll spot something I haven't thought about, but worst case, they'll know what's changing and might get some ideas for their own work.

The next thing I set in stone, is that frankly, we used to have a "blame culture" - people spent far too long worrying about why something went wrong and it simply detracts time from just fixing the problem. So I've banned it. Even if the Board are asking "who screwed this up" - and it's something my team were responsible, my answer is "me, and this is what I'm going to do so we never have that problem again. I'm sorry." On top of this, as I keep reminding the team, there's no such thing as a "silly question."

As a result, the team is 3/5 the size it was, but the value we provide to the business is now well over 15x what we were doing per Sprint. In a few cases, we've turned around bug reports in the Live customer facing system, within 30 mins.

Re your point about tone, and communication style, I couldn't agree more. I've always preferred that I don't leave comments, I have questions, and when I have a question I prefer talking to the person. Lately I've let that slide a bit and been posting questions as comments - but there's nothing to stop me, during WFH pandemic situations, picking up the phone & talking to them. So thanks to your prompting, I'll be doing that.

On your point about changing delivery scope - I don't think the example you state would have been bumped to the next Sprint immediately (in our situation). We would have probably had you continue, and accept that it flows into the next Sprint (unless there was something higher priority/value that you could have done first). That said, delivery scope can, and should change when necessary - locked Sprints is a bad concept for Agile.