Code review is a very important part of the development, so what should we pay attention to review work? How can review work well?
This article provides 10 suggestions, which I think are very instructive
Original text: betterprogramming pub / 10 – tips – to -…
We’ve all seen code reviewed and rereviewed and finally merged after several rounds of changes.
The author would be annoyed if the review students did not approve of the code they wrote, and the review students would think that the author did not receive their opinions well.
However, there are some good methods, not only that depend on the author receiving review feedback well and making changes in a timely manner, but also some good practices to ensure that the review process is less boring and clearer.
Efficient code review is good for everyone because:
- The author can receive your feedback more clearly, resulting in fewer rework changes to the code
- You will provide advice and tips to the author that will influence the overall code specification so that the author will make fewer mistakes in the future
- The code will be merged more quickly without affecting the work of other people involved
- At least there is a quality discussion of each implementation
Please note that this article does not explain how to do code review, or what to look out for when doing so. Instead, we’ve listed some suggestions and rules to guide you through the methods and behaviors involved in code review.
Let’s get started.
1. Always ask why
Let’s take a look at some of the reviews that might exist on review:
- Bad way to name
- This code is not in the right place
- I think there might be a problem here
There are some problems with these comments, most notably the absence of a reasoning process. For these comments, the author can only assume that something is wrong with his code, but we know that “assuming” is a bad behavior, especially in these examples:
- Authors may conclude that their code is incorrectly named and then make a faulty fix, which leads to additional review work.
- Authors don’t get valuable feedback on code naming errors; otherwise, they learn new things and expand their horizons.
What is a better review? Let’s look at an example:
This is not a good name; it should start with a verb that represents an action. In this case there is a suffix indicating something like ‘… ByUserID’, be sure to check out our code specification documentation for more details.
Yes, you just wrote more text, but you now have a clearer idea of what the problem is and how to deal with method naming
Do you know how great you’re gonna feel? The next time the author invites you to code review, you see that they implement the method name as you suggested.
2. Do a thorough
The author of the code can be the most senior person on the team (who knows all the code details), or a new employee submitting the code for review for the first time.
None of this should affect the quality or completion of code review, nor should it make you slack off just because you trust their work.
Code review is not only about ensuring code quality and structure, but also about finding boundary issues or bugs that the author might have overlooked, or even just writing random characters.
Recognizing that these measures are of great benefit to us, it is always more effective to find a bug before the code is attached to the production (or staging) branch than to “find the bug-> report it -> find the cause -> fix the problem and send code review.”
3. Don’t give complete results
As a code reviewer, when you find a problem: It’s not your job to fix the problem code or propose a solution.
Yes, you can (and sometimes should) offer advice or discuss your ideas, but that doesn’t mean you have to offer a complete solution. Assume the following comment:
This method is not very good for execution and should look like this: ```` < code > ````Copy the code
When you do this, the authors of the code will learn very little from your suggestions, and in some cases they will just copy your solution without thinking about it in detail.
Especially if you do this a lot, it’s easy for code developers to become lazy and not think through writing complex applications, assuming that code review people will help find problems and offer suggestions for fixes. If you do this often, don’t be surprised when you are often invited to code review.
In this case, the correct approach is to describe what the corresponding method should look like. You should explain why the original code didn’t work well, and what you would have done, step by step. In some cases, it’s ok to write some pseudo-code, as long as you get the developers to actually think about what you’re suggesting (and not just copy your solution), that’s much better.
4. Don’t postpone review
In any kind of team, but especially for engineering teams, it’s important not to get in the way of each other’s work.
It’s not easy for developers to switch contexts back and forth, and while waiting for a code review of one issue, they’ll have to dive into the background of another issue, and then move back and forth when the last review gets a review to fix.
Review will happen even if it happens quickly, because developers will want to move on to the next task. However, the longer a developer waits, the more tasks he will have to deal with, the more code reviews he will have to do, the more he will have to switch back and forth between branches: re-reading his code (another context switch), resolving conflicts, etc.
Also, imagine that you’re actually blocking the other person’s work, meaning that the other person (the code submitter) can’t continue his work until his code is merged. In this case, they won’t actually work until you finish the final review. The whole thing is inefficient and annoying for both parties (because the code submitter messages the reviewer every half hour to review 🙃 if he feels his work is blocked)
To make sure you don’t compromise on this messy situation, it’s always a good habit to prioritize code review in your schedule. Of course, you may have more pressing tasks to deal with, or you may not always be able to take the time to review immediately, but it is your responsibility to find the right balance between your work and review.
If you have the responsibility of reviewing code for a lot of people, schedule a time slot in your personal schedule each day to focus on that work (or two to iterate faster).
5. Refer to the guide
If your team is large enough or structured enough, you might write some guidelines in the knowledge base as a guide for writing code for your project/code base.
It can include code structures, naming conventions, and many common design methods. If you often write code and review other people’s code, you might be used to these guidelines.
But you should try to re-open the guide for a quick read every few weeks or months, and you’ll be surprised at how many points in code review you’ve forgotten or missed.
If your team does not have such a guide, it is a good idea to write one during review code. In just a few code reviews, you’re bound to come up with a long list of points of concern that you found during the review.
More importantly, such a list of guidelines will serve as the sole standard on your team, ensuring that all code writers and reviewers have something to refer to when writing/updating/reading code.
6. Appreciate good code
Code review isn’t always all bad news, sometimes you’ll see some elegant code that makes you appreciate the author: a piece of logic, some very effective query optimizations, or a great user experience. Praise him.
You can simply comment on the code using nice! Or use longer sentences to describe how you appreciate it. Either way, the person will feel motivated and this will definitely have an impact on teamwork. Also, if the rest of the code isn’t so good, they won’t feel too bad after review.
Most importantly, the author now believes that the code they’ve written is actually good, so they can try to write more of it in the future, or take a similar approach to other problems.
7. Be humble and positive
Over the years, I’ve come across many reviews that are stupid or can be done without. On some bad days, I may have written about some of them.
Such comments are simply offensive and frustrating and do not create any value. If you want to learn more, Gitlab provides some good demonstrations in this area, which sounds very much in line with their company’s values of “positive intent.”
To make a long story short, it pays to be kind in the comments, to respect other people’s time and work, and to assume that they’re doing their best when writing the code. Bragging, shaming, or getting angry in code review will do absolutely no good.
Let’s take a look at some real-world examples I copied from the previous code Review, and note the expression that sets the tone in the sentence:
- Shouldn’t that also be “value”? 🤔
- Maybe you can use “find” instead
- I think the name could be “get…” At the beginning
- With all due respect, “Synchronize” sounds strange here.
- Please add a blank line here
Trust me, saying “with all due respect” or “🤔” will go a long way.
8. Indicate resources
I’m embarrassed to tell you how many times we shared links to CSS properties in code Review during my first front-end days (probably just configured stylelint 🙈), and I believe it even appeared in my bookmarks at one point.
If you know of a resource that can help code writers solve problems, please post a link in your comments. They tend to pay attention to reviews and may even be a frequent resource in their work. If your resources are large and information is hard to find, you can also post specific sections of your company guide.
Perhaps you can point out that authors rely on important resources in the code base. The existing related implementations and components in the past are a good guide for authors who may not know specific parts of the code base if they haven’t used it before.
If you find some code that needs to be optimized and you know of a similar implementation before, don’t assume the author will find it. Point it out in your comments so that the author can compare his/her code to existing code and make improvements or follow the same conventions if necessary.
9. Not just you, but the whole picture
When reading code for review, you must be able to easily understand it. If part of the code is hard for you to understand, that doesn’t mean you didn’t read the code carefully. Instead, it means that the code is not written clearly enough.
If you don’t understand what that block of code does, it may be difficult for other developers to understand when they need to touch that part of the code. When you detect such code, you should not hesitate to speak up. And don’t just ask the author to explain the code block to you, ask to rewrite it.
If you can’t make it any clearer, at least ask the author to add some comments to the code. Feel free to ask questions to understand what the code is doing so you can suggest alternatives and discuss solutions, but you shouldn’t just be satisfied with the responses, you should make sure that the final merged code is easy to understand.
10. Your plan isn’t always optimal
There is usually more than one way to get things done. When you read a piece of code, you might think of another approach and be inclined to suggest it in the comments because it’s a better solution. Sometimes you need to ask yourself: Is this really a better solution? Or is the author’s approach also good, but suggests a solution you use more often?
Unless there is a clear advantage to code complexity or readability, think twice before recommending a rewrite. Reread the code and make sure it fits your team’s conventions and code style, that it does the job well, and compare it to the alternatives you have in mind to see how it might affect code quality. If the author’s approach is fine, but you still think yours is better, make it a suggestion rather than a mandate, and be willing to discuss the reasons and details of your proposed change with the author.
Hopefully, these tips will help you become a more effective code reviewer, saving you, the authors, and the entire team time in the long run. If you have any thoughts on any of the above, or any other suggestions that might come in handy while reviewing the code, let me know in the comments.