4 culture lessons learnt from code review sessions
Not all of my previous companies had the habit of code-review. Yes~ I know how important a code-review session could be (learning new techniques, re-using other member’s methodologies / ideas and more); however personally some code-reviews actually crashed the trust between team members and eventually pushed developers to leave the company. So as a sharing session, I would go through a few company-cultural things learnt in this process…
Negative or even offensive comments
I suppose nobody loves to be spearheaded with negative comments. The fact is, code-reviews are all about validating whether the code change is effective and efficient; thus we are not judging the developer! Somehow, I did met a reviewer (a senior system analyst, just named him as — Tom) who just yell at people “this is stupid!”, “Nonsense”, “Don’t you know what levenshtein distance is?”.
The first thing is — why yell? Alright, the code sucks but that doesn’t grant anybody the right to disgrace somebody.
Code-review, repeat again, is about validating whether a code piece is effective and efficient. Hence, there is no point to offend the developer, rather than yelling, providing positive and constructive comments should be the right thing to do. Plus, we are humans, we all have bias and therefore what seems bad to us might be gold to others. Take an example, we all learnt for-loop and recursive-loop, for most cases we would pick up for-loop as it is easier to code and understood (also debug); and recursive-loop is like a devil. But the fact is, when writing a parser, recursive-loop actually works easier than for-loop. That’s why if we already have bias on certain coding style and techniques, encountering something we don’t get used to or like would bring out the negative comments pretty quickly~ So always stay calm and focus on the code’s efficiency and effectiveness, be constructive.
Back to the story, so what happened next was, the developer (let’s name him — Ben) being yelled got furious after reviewing 10 more minutes and eventually both started to yell at each other until the team-lead needed to stop the session. Ben left the company afterwards.
Management failed to rectify the ill-culture
The story continued, after Ben left the company, the team-lead didn’t do much to rectify yelling-Tom’s behaviour. Unsure about the reason, Tom actually became more and more furious, the yelling behaviour spreaded from code-review sessions to standup-meetings. You know what? Both sessions should be an open discussion on how the code should be developed and what could facilitate a faster development on the tasks, but since Tom is yelling all the time… everybody stops talking in the sessions and of course the team’s trust + collaboration became weaker and weaker. Team members just want to get away from trouble and “safeguard” what seemed to be right for him/herself. People started to leave and the company hired back-fills, but again most of the back-fills left within 6 months.
Doesn’t this sound familiar to you? :) I would say if the team-lead at the very beginning did a fair handling on Tom and Ben’s affair, things would not turn to the dead end. No matter what, the bondings within a development team is very important to the success of the software project / product.
Lesson learnt — if you were the team-lead or manager; do rectify ill-culture within the team. Avoid conflicts to correct something will lead to even more serious issues in the future.
code-review as a Timesheet activity
In another company I had serviced, there is a timesheet system. Not sure how many of you had such kind of experience… I can tell you, management sometimes could not justify how efficient developers are; hence a timesheet system is the solution to jot down every hour what the developers did.
At first, timesheet seems to be the easy and elegant approach; however, this eventually leads to another rabbit-hole.
Long before we had that timesheet system, code-reviews were usually arranged on a Friday noon and the duration was usually 30 minutes to 1 hour. Ever since the timesheet system was introduced, the code-review sessions got arranged like twice per week (Wed and Fri) and the duration extended to like 4 to 6 hours (sometimes even the whole day). Huh? Why?
The reason was pretty obvious, if code-reviews were a kind of quality of work… and we knew that reviewing people’s code was like a social chat activity (assume no yelling)… plus all these hours were proofs on we contributed to the company… why not run a couple more sessions? Yay~~~
After some months, our project was facing serious delays. The management could not find the reason (oh well…) and started to suggested us to work over-time with compensations. Guess what, we did work over-time and earned more for a couple of months. Though the fact was — delays were still inevitable and interestingly even more code-reviews were arranged as the number of bugs increased due to the speedy development. Fortunately, the management found a way to extend the project and at the same time stopped all unnecessary code-review sessions — plus code-reviews were not counted as a valid task in the timesheet, things then got back to the right track.
One weird thing was — 1/3 of the team actually left after stopping the code-review sessions… but we kind of made it to deliver the product before deadline.
Lesson learnt — code-review is important but luxury for some companies; hence don’t push these sessions if your company is not ready for it. Also timesheet as a solution for development might not be suitable sometimes; as developers need to find time to think about the solution / algorithm before coding, whilst all these brainstorming sessions might not even be a valid item within the timesheet system.
Hey I am a cowboy~
In a startup, I had met a couple of talents or even geeks. I know I know, you might want to ask are all geeks hard to communicate with? My answer is “not really”.
Geeks are usually less talkative but that doesn’t mean he/she is a jerk. Instead if you can find the right way to communicate with them, they are just your good old neighborhood. Though sometimes, you would have a chance to meet some “cowboys”.
Cowboy style developers are quite unique, they have talents and they are talkative and full of confidence. Wait a minute, talkative and smart developers~ What do you expect more?
The fact is, during the code-review sessions, cowboys usually don’t listen to others’ opinions. Like Clint Eastwood, guns and horses (maybe also wine) are his friends and not a team. So obviously they would defend all kinds of opinions from the reviewers (as they don’t need/want a team to collaborate). Some cowboys are less defensive and seems to play in-sync with the reviewers however after the session is closed, no code piece is updated according to the comments discussed.
At this point, the management should jam in and make sure that collaborations are made. And for the story, eventually the product manager needed to let go a few cowboys (super talented developers) and keep the family-like culture in the team.
Lesson learnt — code reviews actually could help to spot out who is your “family members”. Among the geeks I have met till now; most of them hate code-reviews, but a small group of them actually enjoys a lot. Therefore, don’t be bias :)
