Code review? No thanks 🙅

Published in

Mercadona Tech

6 min readDec 5, 2023

Hi 👋, for giving a little bit of context, I’m a backend dev in the V6 vertical in Mercadona Tech. Our backend team comprises 3 amazing (and lazy) devs, and we are responsible of managing the dark part of our beloved product: the Stocks App. This app is used in the physical Mercadona stores to control the Stock (reception of trucks, count the stock, report breakages, make donations… and a lot more 😼)

If you want to know more about us, you can check these posts:

Today, I was thinking about the fact that we don’t put more description or info in our PR’s than some random GIF and a name for the final commit (we use squash merge, and every PR that we make finally converts in one commit to our main branch)

If you don’t believe me, I promise that 95% of our PR’s look like this:

But how can we work properly without explaining our code to our teammates? 🤯

Well, I think this is because some factors combined:

Small PR’s

We try to make our PR’s as tiny as possible. They are easier to read, easier to make (and faster to merge), easier to review, easier to rollback… do you need more advantages?

It’s like the first SOLID principle: do one thing. If you make a little change in your codebase, just make one PR for it! (If you usually make significant changes in your codebase, maybe the problem comes because of your slicing).

This is one of the best things we do, and because every time we merge one PR to master it goes directly to PROD (here you can understand why we do this), if it fails, we can easily detect what was the cause, because it’s a small PR, with small changes, and a small piece of code.

Slicing together

Well, a part from the slicing of the tasks that we do every sprint with the rest of the team (product, design, natives…), we also slice every task in backend together.

Before starting every task, we take our time to talk and discuss how are we going to do it, define our subtasks and some times even the naming! We define how we want to do it and then we make them together or we split up if they are really easy or mechanic.

Communication

It’s not only the slicing, we usually work together in person from Monday to Thursday (and we take Fridays as remote hehe), but, even if we are in person or in remote, we maintain a good communication while we are coding.

If we have some doubt in the process (how we will test some code, what is the best naming for a provider or variable, how can we refactor some method, or whatever we aren’t sure about), we just ask our teammates! In that case, we take our time one more time to talk and define what we are doing and why.

Pairing

I think this is the most important thing for us. It’s very similar to the point above. Our communication improves a lot more if we are doing pair programming!

It also has a lot of benefits, like helping us to learn and share knowledge, and also to code better and faster, and with less errors or misconceptions. We code almost everything in pairing, and our bounds are stronger than ever 🫂

I don’t understand this, show me the code

Usually, we code in pairs, and therefore, we don’t even need a review for our PR’s because we have already done them together, but sometimes we code alone, and then we ask our teammates to review that PR.

Usually (and because we have done all the other things I explained before), the review is just an “ok, I like that,” but other times, we doubt the changes.

But this is not a problem! We take our time to review the PR together and discuss the changes, and maybe it’s just an explanation, or perhaps we finally end up doing some refactoring or changes on it (and because we all know about the problem and it’s a small change, it’s not challenging to modify things in them, and it’s also really fast 😎)

Feature Flags

Another thing that we use is Feature Flags. This is one of the keys for us to deploy to PROD quickly. I have an article explaining this here.

Test everything

Of course, we test everything and have our nice, clean, and cool tests to reflect every change we make or add to our codebase (you can see why we do this here.

We take our time and effort to keep our tests clean and understandable, which helps us understand our code and PR’s because usually, one change comes with a new test that explains perfectly what we have done.

Conventions

We are quite a mature team and have a lot of knowledge about our product and project. Over time, we have made some conventions that every one of us understands and knows, which helps us with our code development.

These conventions change a lot while we learn new things or think about how we want to structure and create our code.

These are conventions about how we test our modules, how we name our things, when and why we use factories, builders, inject dependencies, use providers, where to put our files inside our repo, and so on; you know all the things that I mean 😉

But yes, we have some info in our PR’s

As you can see in the first image, we try to put a good name that describes the final commit, and we also have the changed, added, fixed, removed, deprecated, and security tags. We add this to every PR to easily find them when checking some old code.

Last but not least, we also added the Jira reference to the ticket we are working on in case we want to see the User Story in the future (and find some possible explanation about why we did that).

All of this is so cool but… is it really worth it?

Well, I am not going to try to convince you that this is the way to do things, and thanks to being a data-driven company, I’ll show you some data and metrics of our results in the backend for our last month, October of 2023:

In summary, you can end up with things like this when you put together all of these practices (I know it’s hard, and it usually comes with a strong company culture). I think not having to comment on your PR’s is pretty similar to not having to comment on your code because you have done many things before that make everything easier (apply SOLID principles to everything in your life 😉).

As you can imagine, this has not always been like this. We have been learning a lot along the way and improving everything we could, always trying to work better together, and we have much more to learn!

So… yeah, I’m very proud of us, and I think we are correctly doing things (I also know that we have a lot of things to improve).

Of course, this is working for us because of our team conditions (we are only 3 backends, and it’s easy to apply all of these things), and maybe with bigger teams or another structure, it is not so easy to use. This is not the ultimate truth on how to do things, and it’s just a way that, at least for us, works pretty well 🙂

Last, this is happening because there is a lot of hard work and effort behind it, just say thanks to my team for making this possible. I know there is still a lot to come!! 💚