Original link: https://www.hwchiu.com/read-notes-54.html
Title: “How to Provide Professional Code Review Opinions”
Category: others
Link: https://medium.com/@yar.dobroskok/how-to-review-the-code-like-a-pro-6b656101eb89
The author straight to the point mentioned that if there is no code review culture in the team, please ignore this article directly.
When the team really has experience in code review, they will have the opportunity to improve the entire code review process through some concepts shared in this article, which is efficient and time-consuming.
The author believes that a good quality code review can help the team to bring the following benefits
- Avoid merging buggy, unreadable, inefficient code into the project
- Developers can share knowledge with each other
- Get various opinions on implementation
- Ensure consistent coding style across the team
In order to fully import the above concepts into team projects, the author shares some of his commonly used concepts and moves
Prepare a Checklist in advance
A good review process is to have a checklist. This list describes the rules that each code merge “must” meet. It is also a rule that the team attaches great importance to. This list has no absolute standards. It is mainly based on Team to think about what is most important, for example
- Whether the content and name of Branch, Commit conform to the specification
- Is the code readable enough
- Codesytle and whether naming conventions fit the team culture
- Does the folder/file structure fit the team culture?
- Does it contain relevant tests?
- Are the documents prepared together?
The point of this list is to include only those items that are considered very necessary and important, otherwise the whole list is not very meaningful.
Automate the above checks as much as possible <br />After preparing the aforementioned checklist, the next step is to find a way to automate the above-mentioned checklist, such as
- Check codesytle through linters
- Run tools like SonarQube, Codacy, etc. to help check for potentially inefficient or vulnerable code
- Run automated tests through relevant frameworks and get relevant coverage reports
When there is a way to automate these operations, the next step is to think about when?
- For some quick checks, such as linter, beautifer and other tools, you can consider integrating them into pre-commit hook/pre-push Git hook and other time points to run so that developers can quickly check simple errors
- For some time-consuming checks, such as analysis tools, tests and related construction processes, these can be put into the CI pipeline to run
After everything is ready, it can be integrated into the entire git tool. For example, only the PR that passes the CI pipeline needs to be reviewed. If the automated test cannot pass, it is the developer’s responsibility to It’s done, everything is ready to start the last step
- Manual intervention review *
When starting the manual review, because the aforementioned automated process has helped to check a lot of things, the focus at this time is the operation logic.
If possible, the author suggests that the review should be discussed with the developer directly instead of looking at the code conjecture slowly, which can avoid the ineffective time spent in back-and-forth communication. In addition, the developer can more clearly explain the reasons and considerations behind all implementations.
The author also recommends using an IDE for code review. Many powerful IDE functions can help developers to review code more efficiently, such as quickly finding the declaration point, the called point, and the appearance of the entire data structure, which can be saved. quite a while
Finally, the most important thing is that the size of each PR should not be too large. This is actually a principle that has always been pursued in the Linux Kernel. Excessive revisions have too many files to see, and there are more potential incompatibility issues. Pay attention to this. It is a heavy burden for developers and reviewers, so if possible, splitting the revisions into several meaningful PRs and reviewing them separately will make the overall process more efficient, and at the same time, it can also avoid the possibility of being too hard to read when there are too many files. Stamping behavior of direct brainless +2
personal information
I currently have Kubernetes-related courses on the Hiskio platform. Interested people are welcome to refer and share, which contains my various ideas about Kubernetes from the bottom to the actual combat.
For details, please refer to the online course details: https://course.hwchiu.com/
In addition, please click like to join my personal fan page, which will regularly share various articles, some are translated articles, and some are original articles, mainly focusing on the CNCF field
https://www.facebook.com/technologynoteniu
If you use Telegram, you can also subscribe to the following channels, where I will regularly push notifications of various articles
https://t.me/technologynote
Your donation will give me the motivation to grow my article
This article is reprinted from: https://www.hwchiu.com/read-notes-54.html
This site is for inclusion only, and the copyright belongs to the original author.