Code review horrors

Questions
#1

WIN_20210107_11_15_13
WIN_20210107_11_15_13700×560 75.8 KB

A bit of a rush today, so just a quick question and motivation.
For me this question is easier than for most, because I’ve coded in lower level languages in the distant past, but. Do code reviews meetings in your team give you the horrors?

I have found what I hope is the relevant bug in my bug mug (you can get one of these on the MOT merch store). My little admission here, is that my team uses so many new languages, that I’ve never used that I often cannot read the code in much depth. BUT It does not stop me “playing duck” during code reviews. If you don’t participate in code reviews, and would like to learn what a “duck” is, chime in below.

3 Likes
#2

Are you asking about experiences with bad code written by testers?

If yes, then I have seen some genuine horrors. Some of the horrors are committed months ago by the guy in the mirror. Some by testers who are bad, were not trained, did not have enough time or by a combination of those factors.

Btw, by playing duck, did you mean “rubber duck debugging”?

2 Likes
#3

Oh no, No, I’m talking about the product code. I should have explained my context, this is what happens when you rush. Do you sit in on team code review meetings or open up the code reviews system for hotfixes that need to ship quickly and you thus won’t have a tonne of time to regression test them, because they are just “one-liners”?

ninjaduck
Oh yes @raghu The rubber duck. Basically you are supposed to just sit quietly while the developer walks you through the problem and through the code, staying silent the whole time and just nodding.

Some parts of this are easier to do remotely, especially in lockdown. Most teams are co-located, something like this in a physical office. We are all on Slack/Teams/Zoom, so you don’t really have to pay attention. So when your team has a detailed design meeting, or code review walk-through just join, even if you would not normally want to, because you can just carry on working in the background on your testing job, and just be there in the “room”.
team
By just trying to join in, testers encourage code review in their teams. How has this worked for other folk?

1 Like
#4

A few years ago, I had an unbelievable amount of resistance when I suggested testers should be involved in code reviews. This was at a time when CI/CD wasn’t even thought of in my org and sprint builds were only a few main features so code reviews tended to be large amounts of time. I felt we should be involved to help our understanding of the feature but also to help find the small things which may otherwise be missed. I blogged about it here:

3 Likes
#5

We make a genuine effort to review all changes for Product Code, Test Code and Infrastructure as Code. I’ve rarely seen anything merged with insufficient scrutiny.

I often take part in code reviews. I do my best and I’ve found a few things.

We typically do code reviews as an asynchronous activity via GitHub making comments or requesting changes. Occasionally we will chat on slack about interesting or contentious points and often pair or have group calls to make progress or fix things.

2 Likes
#6

If you are really good at code, and can provide value to the code review meeting by pointing to the issues in the code, then you should. If you can’t provide value, but just be there like a ‘rubber duck’, then you might become the ‘Elephant in the room’!

2 Likes
#7

I wish I could write brilliant blog posts like this ^^ myself. Reminds me of the age old question, when does testing stop, and when does testing start. It often starts before code gets committed to the repo.

Everyone hopefully knows, I was a programmer in my previous life, so code reviews are easier for me to be active in, but my team do use languages I’ve never learned, and they use advanced features of the language too. So yes, it’s OK to not understand the code. But I hope that by encouraging the reviews, that we make sure that they happen. And by listening in, we can get hints as to which features of the product are actually the most tricky and thus most likely to be good bug hunting grounds.

If your team is struggling with code complexity, it’s not rude to put your hand up at a point before an argument in your team starts, and say “Hey, what if we re-wrote this code differently, would it be easier to maintain and progress this?” . Even if you don’t know the answer, you might find you gain an ally if you time these right.

3 Likes