Code Review

Smart Coupangpay developer life

I would like to work for a company with many meeting rooms.

In February 2016, I joined Coupang. Looking back, I think there was a lack of meeting rooms. I had to do a code review, but there were no conference rooms, and time just kept ticking by. In the end, the method we chose was to use the empty walls of the office. It's like this. I should have taken a picture.

Sometimes there were developers crossing the code review screen.

It was amazing. Why not do it if you don't have a meeting room or if the scope of the code change is small? I'm busy, but why is this company so obsessed with code reviews? I've been developing for many years, and I've been to many companies, but this was the first company that had a culture of code review. Existing companies mainly proceeded in the order of development / QA / release. In fact, I absolutely hated showing my code to my co-workers and having other co-workers fix my code.

A very rare example of showing code was:

When development doesn't go well, when I want someone to help me

When there is a problem after deploying but cannot find the problem

When introducing a new library. "Guys! This is how you use it. The code gets really short!"

What's the point of doing a code review? I'm busy, can't I just distribute it?

When you go to Rome, you must follow the Roman law. I had no choice. 

From now on, I'm going to talk about what I experienced and realized through the culture of code review after joining Coupang.

Even doctors cannot cure their own disease.

After completing the development, they usually test in their own way or verify the development through the test code. However, no matter how many tests the developer has done (even there are many developers who do not test), they have no choice but to do it subjectively. After doing a code review, someone found this part.

"Wow~~ I never thought something like this would exist!"

Notify co-workers of code changes.

When I first joined the company, I searched for documents with the desire to quickly adapt to the work. There were few. There was a wiki, but I wonder... it feels like it's not organized very well. Documents using diagrams were still worth seeing. There was a diagram of the overall system structure, and there were documents that well organized the payment flow. One thing that was amazing was that when I asked because I didn't know, the existing engineers knew too well. "Are these engineers memory geniuses?"

I gave up on looking for documentation and started looking at the code and trying to understand the whole system through diagrams. The code was readable. When you think about it, code is great documentation. After all, aren't developers accepting requirements and sublimating them into code!

Through code review activities, inform why this code was written, which parts were modified, and which parts were newly created. Through these activities, I was able to understand what tasks are and how the system works.

The team's code, not my code

Through code review activities, my code becomes the team's code. Even if I didn't develop it, it becomes the team's code when the code review is over. Through code review, we have already identified why this code was created, identified defects in the code, and already know what kind of error message will appear if this code has a problem. And these small codes are gathered to complete a system. Ownership of the code naturally arises. In the deployment stage, there is a process called Canary. In this stage, there is a process of deploying to only one actual operating server, receiving traffic, and monitoring whether there are any errors or server problems. I understand the code to some extent and know when it will be released, so if a problem occurs at this time, I will immediately go through the RollBack process. Of course, the deployers monitor it, but the team members know it to some extent, which helps them figure out what went wrong later. I hope you understand that it is the difference between the code that only you know and the code that your teammates know.

Learn through reviews.

I'm not talking about new technologies or techniques. When doing code reviews, tech leaders, managers, and fellow developers who understand 60-70% of the code participate, unless they are really busy. They can look at the code itself, but they have the ability to review the problem in three dimensions in the overall context. Not because they are outstanding, but because they know the whole thing to some extent. Fellow developers look at the code and the manager gives advice based on business or history.

About these stories come and go.

Too many implementations in one function can make it difficult to see the code later.

Should we write this kind of code? Wouldn't it be a good way to discuss with the person in charge and share only the data?

There is already a Utility function for changing the date format, so I'd rather not implement it and use the Utility function. That way, I think it's consistent and easy to maintain.

I heard at the Leader meeting that a payment authentication method could be added. I hope this part can be developed to be scalable.

I don't think payment should fail just because there's a problem with this part. It's an additional feature, so it's better to separate it into a separate transaction. In case of an error, it'd be better to leave an INFO-level log. 

It can be a huge benefit for Junior Developers and developers who have just joined the company and don't know the whole thing. Rather than taking advice from one close colleague or mentor, it actually helps a lot to have multiple people reviewing it. You can save time by being able to make decisions right away, and learn more deeply by showing other codes for parts you didn't know. After all, when these reviews are repeated several times, the ability to understand the code increases and confidence is gained. Being confident in your code increases your productivity.

Is there an order for code reviews?

1. Make an appointment

Nothing special. After completing the development, review the meeting times of your colleagues and set up a meeting at a time that allows everyone to gather as much as possible. If the amount of code is small, the reviewer will inform you of the name of the Git branch or tell you in advance during scrum that you will do a code review for a certain part. When the amount of code is large, a separate wiki document is created or a diagram is drawn and a link is written in the invitation email. Depending on the amount of code, it can be as short as 30 minutes or as long as 1 hour.

"I'm reviewing the code. There was a bug in calculating the amount when using Coupang Cash, so I fixed it. Please attend."

2. Concept Code Reivew

Engineers have to develop, but sometimes they don't know how to start. Or, when they want to develop a scale that cannot be completed within a sprint (2 weeks), Concept Code Review is recommended for tasks with many dependencies with other teams.

(Concept Code Review step is not required.)

It's a place to ask my colleagues for my thoughts. Sometimes engineers actually write a lot of code and then go back to development because they didn't get the direction right. what a waste of time?

We usually ask for a concept review by organizing it as follows.

Wiki - Organize the requirements you understand and what to do if you implement them

Diagram - Draw in advance how you will interface with other teams.

A simple sketch of the cache usage API

Make a sketch of how the back office functions will be built. Some developers even plant Easter Eggs. :)

Back office function sketch

Developers seem to enjoy Concept Code Review time more in my opinion. It is not a place to decide the correct answer, but a place to decide which method is better. Developers' imaginations can be exercised and they can talk in a more free atmosphere.

3. Code Review

If all your colleagues are gathered, simply what part is the code to solve? Explain and share the editor screen to review. Since we work almost from home due to corona these days, we are using online video conference to share codes.

4. Update code & Deploy

If there are many modifications made in the code review process, repeat step 3 again. If the changes are minor, notify your colleague that the changes have been reflected and ask your colleague to merge the code. The colleague who received the merge request checks and merges. Then, the manager goes through the process of checking the code one more time, and the deployment proceeds.

Important story

How did I adapt to this culture with no code review experience?

How could the Coupang Pay organization continue to maintain this culture?

Both goals match.

Reviewee

Reviewee want to verify that the code is written according to the requirements. (most important)

Reviewee want to know about flaws in the code that he/she don't know about. stability during deployment.

Inform the team of what has been implemented.
"With this update, if a request to use the cache comes in from a user who has withdrawn, there will be no error."

Reviewers

I wonder how it was developed.
"How will existing processes change with this change?"
"If we have this function in our back office, we will be able to recover quickly in the next failure!"

The manager or tech leader of the team must understand the business in charge of the team or even the code level to some extent. So I try to participate in code reviews whenever possible. What's better than not paying attention and suddenly asking for code?

To appreciate master's code :) In the early days of the company, I was curious about how other colleagues developed codes and what ideas they had.

See if the code is operational. The process of becoming team code rather than individual code

Focus on the essence without greasiness.

Code review is also a human job. When reviewing, you may unintentionally hurt the other person's feelings.

Focus on the essence of what you want to resolve, not your personal feelings.

We need a gentle expression of caring for each other.

We know that good quality code (operable code) can guarantee our warlabel. :)

 

Concluding

I think it will be helpful for those who are hesitating on how to start a code review.

I wrote it in the easiest language possible, but if there are any terms you don't know, please leave a comment. :)

Thanks for reading.