A collision of ideas caused by code review

[[236712]]

At the team's iteration review meeting today, there was a small topic about the issue of "problems in code review records not being closed in a timely manner".

The reason for this is that our team has been practicing daily code review for more than a year. During the review, everyone will raise a lot of questions and modification suggestions for the code submitted the day before, and then a quality tracker will record these questions and the person responsible for modifying them in a table on the wiki page. If the person responsible for modifying the code closes the code problem, he will log in to the wiki and check the box of the relevant problem item, indicating that the problem has been fixed.

It seems that the rules and personnel are clear. However, from observations in recent months, the repair and closure of problems found during walkthroughs are not ideal. Usually, many problems remain there and are not checked off at the end of the month. I usually play the role of a "gatekeeper" and urge them to check off at the end of the month, but frankly speaking, the effect is not good and the result is half the effort.

Since the project and department quality departments will pay attention to the implementation of code walkthroughs for each team, although they spend a lot of energy on code walkthroughs every day, the external perception and result statistics show that a lot of problems have not been closed in time and the problems have not been closed.

What went wrong? How to solve the puzzle?

Although it seemed like a small issue, everyone started to speak out and a unique democratic discussion broke out.

Viewpoint 1: The problem of quality supervisors.

Quality supervisors not only need to record, but also urge relevant development managers to make timely modifications, so that there will not be so many problems left at the end of the month.

Friend A who agrees: The quality tracker did not perform his duties well - By the way, does anyone know who the quality tracker of this iteration is? Forget it, I’m afraid the quality supervisor himself doesn’t know, and the responsibility has not been assigned to anyone.

Partner B who agrees: Then let’s take a look at the iteration rotation table. Starting from the next iteration, let the tracker fulfill his responsibilities - you may as well write the name of the quality tracker on a sticky note and stick it next to the big screen.

Opponent C: This is not a problem of quality trackers, but a problem of personal initiative and awareness. If this plan is followed, the quality tracker must not only record, but also urge others to make changes. If it is not enough to urge once, he must urge twice. The quality tracker is very tired. Let's take a look at which few people did not make changes each time?

Ding, a neutral friend: Everyone is smart and has their own way of doing things. We should trust their professionalism. Don't make everyone confrontational. It will be too stressful.

Viewpoint 2: It’s not the quality tracker’s problem, but the person who writes the code’s problem.

Friend A who agrees: Whoever creates the problem should be responsible for cleaning it up. You should be proactive. It's obviously your problem, why should the quality tracker pay for it?

Partner B who agrees: I modified the problem immediately after the walkthrough. Today's work should be done today. Since the record of the walkthrough is approved, the students who wrote the code should solve the problem.

Partner C who opposes: First of all, I do not agree with all the problems recorded, such as variable naming, spelling errors in class names, changing from inheritance relationship to composition relationship, do these have to be changed, will there be problems if they are not changed? Have the problems recorded in the wiki been approved by the parties involved, or have the majority agreed on the spot.

Secondly, there is a type of question that belongs to the discussion type, which does not require code modification. It is also recorded now. Is it suitable to be recorded in the code review wiki? These problems are time-consuming and cannot be closed for a while. They are usually important but not urgent.

Answer to Student D from Buddy C: Both suggestions are very good. The content of the record should be what everyone agrees on. If there are any questions, check and confirm them on the spot before recording. For non-code issues, derived business clarifications or business discussions, it is best to record them in a separate notebook so that they will not be confused with code modification issues.

Viewpoint 3: None of them are correct. The problem is the current approach.

Friend A: Why not have a quality tracker record it? Let the coders talk and record it themselves during the walkthrough. The problems they record themselves are definitely recognized and they will definitely modify them.

Buddy B: Frequent rotation is not a good idea, and the person explaining needs to be focused, so it is not recommended to record while explaining. It is better not to rotate the quality tracker, and let me record uniformly, and ensure that everything will be checked before the end of the month.

Friend C: Amitabha, I just can’t stand it anymore… How can such a small matter become more and more complicated?

Friend Ding: As long as it involves people, there is no such thing as a small matter. Because people have different cognitions, behaviors, and habits. In addition to the differences in each person's mental model, there are also the complexities of relationships between people and groups. If you only look at the problem from the perspective of "I", it is inevitable that you will find it difficult to understand the complexity behind it. When you encounter a problem, you should look at the problem from more perspectives and analyze the problem. Don't rush to draw conclusions.

Friend C: Yes, we stick to the facts. After a heated discussion, we are still good friends.

The conclusion without a pause

Although this issue has not yet reached a generally accepted conclusion after seeking common ground while reserving differences, I think it is good to be able to discuss it in an open manner so as not to fall into the limitations of individual thinking. As Steve Jobs said, stay hungry, stay foolish. There is no right or wrong in opinions, but whether they are suitable and close to the actual situation of the team.

Adhering to the agile spirit, we can embrace change, try out several small improvements mentioned after brainstorming, and then see the results. If it’s good, we’ll keep it, and if it’s not good, we’ll make further adjustments.