💾 Archived View for gemini.abiscuola.com › gemlog › 2023 › 08 › 08 › please-dont-do-mandatory-code-r… captured on 2024-03-21 at 14:58:41. Gemini links have been rewritten to link to archived content
⬅️ Previous capture (2023-09-08)
-=-=-=-=-=-=-
It's the best way to be sure reviews will become completely useless.
Everybody know that, these days, the IT industry, is full buzzwords, dubious "best practices", inflated titles and, in general, a lack of average competency in the craft.
One of the areas where an army of consultants like to sell their garbage, is about "how to do software development". I believe I already discussed this in the past on this blog, probably related more about why I don't use git and similar topics. But I never discussed others, perceived, best practices, or "how we should develop software" in general. I'm easy at ranting about software quality and whatnot, and many of you probably wants me to put my, metaphorical money, where my mouth is.
Software is devolving, and it's bad
The industry has all of those best practices: Agile here, pair programming there, extreme programming before and the future, probably, will be called along the lines of "bungee programming", or whatever. Code reviews are one of those perceived best practices and, while I strongly believe a good culture of code review is extremely important, the act of forcing them down senior programmers throat is just going to piss them off.
Let me try to explain with a real example.
At $DAYJOB we are a small team. Excluding the tens of testers not directly working with us, but banging on our code, we are a total of seven people, with the following roles:
Try to guess which role I'm in? What? Wrong!!! I'm the only DevOps engineer for the whole team. Sometimes I take care of the release engineering part when the collegue normally doing it is not available. But in general, I'm the one developing tooling, automation, scheduled jobs, general scripting and programmning and similar things for the whole team to use, or for the main application to rely on. It's me, with myself, doing this kind of work. Well, guess what?
I must have the code reviewed by "somebody else" before merging, because the company I do consulting at forces this behaviour, through it's CI/CD process. They call it "following the 4 eyes principle". However, I don't have another pair of competent eyballs to send my code to for review, and making this process mandatory, does nothing more than having a bunch of monkeys (including myself), mindlessy clicking on "approve".
....click....click....click.....click....
Or it happens that somebody sends me a merge request for review, about some complicated part of our C++ codebase. How am I supposed to review a small part of a codebase for our massive system, when I don't even work on that? In general, given that I'm not that stupid, I reject the request, telling people that, the fact that I don't work on that codebase, should give them a clue about asking somebody else. But nonetheless, this still happen, maybe nobody is around, the manager is pushing, people wants to have their 15 minutes of glory and I cede, sometimes, to this bad practice.
So much for "code review".
Ok, ok, I hear you. "We do code reviews wrong", "we should take them as learning lesson", "it's a way to share knowledge". Whatever.
The truth is that, all this happen because those "best practices" were forced down our throats from the top, and when the process to change a printf is:
Rinse and repeat for every, single, minor thing you must do, everybody will become sloppy, due to the feeling of just being a cog into a machine. God. Just thinking about that makes me cry. Combine that with the idiotic branching strategies the ones who knows better decided to adopt, and you have a perfect recipe for making people even more sloppy and careless. In my experience, it happened at every, single, company, I worked at. Instead of being a powerful instrument a professional can leverage when they feel there is a need to, it just become a routine burden. Nobody will take code reviews seriously, and nobody will take the time to do them properly. Because doing a good code review is an, extremely, time-consuming activity.
I strongly believe that, as professionals, we should have some freedom of movement on the matter. Give your teams some guidelines, in particular to help the most juniors member to take a good decision, but in general, it should work like that:
I would like you to notice the importance that, personally, I feel should be given to the people in the team. Let, them, do, their, work. This way, if they call for a code review, the other team members will know that, more often than not, it will be for a very good reason and the review will be taken seriously, discussions will probably happen and it will be a better use of the review time, in this case. Let people be themselves. All this forced collaboration, all those ceremonies does not help a team to become more "cohesive" or more "collaborative", it just pisses off every one of it's members, in particular the most experienced ones.
In my specific case, having others being forced to review my work, doesn't make any kind of sense. They are going to be frustrated because of this! Almost nobody in the team knows bash well enough, or perl, or tcl, or awk. It's just me and that's why my collegues, when asked by me to review something, just approve mindlessly 99% of the times, because they don't have a clue about what my job is. Sure, we do stand-ups, but, to them, I'm talking vogon and they probably don't give a crap, considering our jobs does not even overlap in any meaningful way.
What about open source?
During the years, and after experimenting with a bunch of different approaches, I went back to the basics for my current project. The classic old way of doing software development. For every version:
For the project I'm working on right now, there is another developer participating. So, essentially, a team of two. The mantra is:
The main points then are:
Everything else, in my view, is just a burden. Of course, we don't work on safety-critical stuffs. That's a kind of industry that has a series of totally different processes, it's much more rigid and requires a lot of design up-front (that's a good thing).
Based on how I decided to work on this project, the code review part is covered by the phase between "alpha" and "beta". An alpha, in our case, is feature complete, and it's being tested internally. A beta is ready for the public and it should be just a matter of some minor cosmetic changes before the final release. Is it slow? It doesn't looks like, to be fair. At least, the phases are well organized and spaced. I'm pretty happy about this approach. Right now we are near the alpha, we are working on cleaning-up the code, improving the comments and it's documentation. Fixing bugs if they arises. You have some constraints about what you are allowed to do at any given point in time, but you still have some freedom about how to attack things. There are no mandatory stuffs, just limits about what you can do during a given phase.
The tools are levers you can choose to move based on YOUR judgment. You are a professional and if you need baby-sitting with mandatory things, it means you are either too junior, or you are not a good programmer and you must be replaced.
Let professionals have a choice, please. They'll be happy, more collaborative and more effective, in the long term.