如何进行代码审查
As a code reviewer, you have the power to approve any code, and along with that comes the responsibility to make sure that the code is in good condition. In this article, we will go through a listing of questions and points that could help code reviewers focus on what matters during a code review.
作为代码检查者，您有权批准任何代码，并且随之而来的责任是确保代码处于良好状态。 在本文中，我们将列出一系列问题和要点，这些问题和要点可以帮助代码审查者专注于代码审查期间的重要事项。
If you are reading this article, you might find the following article helpful
如果您正在阅读本文，则可能对以下文章有所帮助
Here are some of the questions that you can ask yourself when reviewing a changelist (pull request)
这是您在检查变更列表(拉取请求)时可能会问自己的一些问题
设计与代码相关的方面 (Design related aspects of code under review)
- Is the code well-designed? 代码设计合理吗？
- Does the code demonstrate low coupling and high cohesion? 代码是否显示出低耦合和高内聚性？
- Should the code be moved elsewhere? 代码应该移到其他地方吗？
- Does the code interact well with the existing code and features? 该代码是否与现有代码和功能良好地交互？
- Functionality-wise, does the code do what the developer intends to do? 从功能角度讲，代码是否按照开发人员的意图进行？
- Is the code written well-enough for both end-users and future developers to maintain and extend upon? 该代码编写是否足够使最终用户和未来的开发人员都能维护和扩展？
- Is the code too generic and over-engineered? 代码是否太通用且设计过度？
- Are there any edge cases, i.e., concurrency, deadlocks, or race conditions? 是否有并发，死锁或竞争条件等极端情况？
- Is the code relating to the web layer, business logic, and database in their appropriate layers? 代码是否在其相应的层中与Web层，业务逻辑和数据库有关？
- For backend development, do the endpoints return Data Transfer Objects (DTOs) or the business model (Entity)? 对于后端开发，端点是否返回数据传输对象(DTO)或业务模型(实体)？
仔细检查代码中的任何依赖项更改 (Carefully review any dependency changes in code)
I do not suggest that you re-invent the wheel and avoid dependencies all-together however, I do strongly recommend that you think twice before adding a dependency to your codebase.
我不建议您重新发明轮子并完全避免依赖关系，但是，我强烈建议您在将依赖关系添加到代码库之前三思。
The future of open source projects is not in our hands, and their trajectory could affect products that rely on them.
开源项目的未来不在我们手中，它们的发展轨迹可能会影响依赖它们的产品。
For example, if a dependency is not maintained anymore and there are security or other types of vulnerabilities then you are stuck with it. Depending on the dependency, a majority of times it is really complex to remove or replace a dependency from a production system without affecting the users.
例如，如果不再维护依赖关系，并且存在安全性或其他类型的漏洞，那么您将受其困扰。 根据依赖关系，在大多数情况下，从生产系统中删除或替换依赖关系而不影响用户确实很复杂。
Another example, the more the dependencies the higher the maintenance of the product and this is because upgrading dependencies is a bumpy ride and depend on quite a few factors
另一个示例，依赖关系越多，产品的维护性就越高，这是因为升级依赖关系是一个坎a的旅程，并且取决于很多因素
- How outdated the dependencies are? 依赖关系有多过时？
- Are there any breaking changes in dependency versions? 依赖版本是否有重大更改？
- Are there official migration guides? 有官方的移民指南吗？
- Is the dependency being forked to accommodate the product’s custom needs? 是否为了满足产品的自定义需求而派生了依赖项？
All of the above decides how complex, and expensive the upgrade process is going to be and how would it affect your customer base.
以上所有因素决定了升级过程将变得多么复杂和昂贵，以及它将如何影响您的客户群。
Develop an approval process for new dependency requests and define which team will own the dependency and maintan it regularly.
为新的依赖项请求制定批准流程，并定义哪个团队将拥有该依赖项并定期进行维护。
Here are some of the questions you can ask yourself as a code reviewer
以下是您作为代码审查员可以问自己的一些问题
- Can the addition of the new dependency be justified? 可以证明添加新的依赖项是合理的吗？
- What are the pros and cons of these new dependencies? 这些新依赖项的优缺点是什么？
- What other transitive dependencies are being pulled via this new dependency, and will there be any conflicts with existing dependencies? 通过此新的依赖关系还提取了哪些其他可传递依赖关系，并且与现有依赖关系是否存在任何冲突？
- When upgrading/dropping an existing dependency, are we confident there will be no runtime exceptions directly from code or indirectly? 在升级/删除现有依赖项时，我们确定直接代码或间接不会存在运行时异常吗？
- Is the project active and maintained? 项目是否处于活动状态并得到维护？
- How many open issues are there? 有多少个未解决的问题？
- What is the test/code coverage? 测试/代码覆盖率是多少？
- How many active contributors are there? 有多少活跃的贡献者？
- How old is the project, and are there any alternatives? 该项目有多大历史了，还有其他选择吗？
避免不惜一切代价分叉和更改库 (Avoid forking and changing libraries at all cost)
Forking and changing an open-source library should never be an option, and that’s because it is too expensive to keep the forked version of the library in sync with the original.
分叉和更改开源库绝不应该是一种选择 ，这是因为将分叉版本的库与原始库保持同步太昂贵了。
The biggest challenge with forking a library is keeping the fork in-sync with the original library as it evolves over time.
分叉库的最大挑战是，随着时间的推移，派生与原始库保持同步。
Let’s say, you forked Project A and added a bunch of code to support your product’s needs. Fast-forward a few months and years down the road, Project A has moved on from when you forked it and there are a bunch of new versions and you would like to upgrade Project A. The challenge is to make sure that your custom code works with all future versions of Project A, without breaking anything. It’s almost always a bumpy ride. Good luck.
假设您分支了Project A，并添加了一堆代码来满足您产品的需求。 前进了几个月又几年，项目A从您创建分支以来就已经进行了，并且有很多新版本，您想升级项目A。挑战是确保自定义代码能够正常工作与所有未来版本的Project A一起使用，而不会破坏任何内容。 几乎总是颠簸。 祝好运。
If you must fork, then make sure to keep your fork as minimum as possible and do your ultimate best to contribute back your changes to the original codebase.
如果必须分叉，请确保将分叉保持在最低限度，并尽最大努力将所做的更改归还给原始代码库。
- Is the code forking an open-source library? 代码是否是一个开源库？
- Can the fork be justified? Why are we forking? Is it worth it? 叉子可以辩解吗？ 我们为什么要分叉？ 这值得么？
- What alternative options do we have instead of forking? 除了分叉之外，我们还有哪些替代选择？
- Are the original code and forked/changed code on separated commits? 单独的提交是否有原始代码和分叉/更改的代码？
在代码审查期间提防技术债务 (Watch out for tech debt during the code review)
Tech debt is almost always the result of a quick and dirty approach of coding a fix or feature due to time constraints.
由于时间的限制，技术债务几乎总是由于快速而肮脏的方法来编写修补程序或功能的结果。
Technical debt (also known as design debt[1] or code debt, but can be also related to other technical endeavors) is a concept in software development that reflects the implied cost of additional rework caused by choosing an easy (limited) solution now instead of using a better approach that would take longer. — Wikipedia
技术债务(也称为设计债务[1]或代码债务，但也可能与其他技术工作有关)是软件开发中的一个概念，反映了现在选择一种简单的(有限的)解决方案所导致的额外返工的隐含成本使用更好的方法将花费更长的时间。 —维基百科
It might be difficult to avoid tech debt for new companies that do not have the financial and technical resources available to them and this is the phase where a huge volume of tech debt is added to the codebase because, the objective is to get a product that works in the market, not built a perfect product.
对于没有可用财务和技术资源的新公司来说，避免技术债务可能是困难的，这是向代码库添加大量技术债务的阶段，因为目标是获得能够在市场上运作，而不是打造完美的产品。
There is no excuse for adding tech debt to an established product that has a healthy customer base, financial and technical resources. Such companies should focus on addressing the tech debt that was added in the earlier stages of product instead of adding new tech debt.
没有任何借口在拥有良好客户基础，财务和技术资源的既定产品中增加技术债务。 这些公司应专注于解决在产品早期阶段增加的技术债务，而不是增加新的技术债务。
- Do the new changes degrade the qualify of the existing code? 新更改是否会降低现有代码的质量？
- Do the new changes make a method/class candidate for refactoring? 新的更改是否使方法/类可以重构？
- Does the changelist (pull request) introduce tech/code debt? 变更列表(拉动请求)是否引入了技术/代码债务？
- Are there any duplicate blocks of code that could be made reusable? 是否有任何重复的代码块可以重用？
测验 (Tests)
During the code review, tests are probably the one area of the pull request (changelist) that would get skimmed through or ignored. Simply because a test is passing it does not mean that it is a valid test.
在代码审查期间，测试可能是拉取请求(变更列表)中将被忽略或忽略的区域之一。 仅仅因为测试通过即可，并不意味着它是有效的测试。
- Are the tests correct? Do they fail when the code is broken? 测试正确吗？ 代码损坏时它们会失败吗？
- Do the tests cover all branches (execution paths) and instructions in the code? 测试是否覆盖代码中的所有分支(执行路径)和指令？
- Are the tests independent of each other? 测试是否相互独立？
- Are the tests easy to understand and maintain? 测试容易理解和维护吗？
- Does this feature need to be tested under load to make sure that it scales well and that there are no memory leaks? 是否需要在负载下测试此功能，以确保扩展性良好并且没有内存泄漏？
添加无效代码 (Adding dead code)
Sometimes the developer will say, well, we don’t need this block of code now but, maybe we will need it in the future. Or, a developer would make a method so generic with the hope that it covers future requirements.
有时开发人员会说，好吧，我们现在不需要此代码块，但也许将来会需要它。 或者，开发人员将使方法如此通用，以期满足将来的需求。
Let’s cross that bridge when we get there. Don’t add dead code and hope that it resurrect in the future.
当我们到达那里时，让我们越过那座桥。 不要添加无效代码，并希望它在将来复活。
Such dead code should not be approved because there is a very high chance that the code is never used in the future or it won’t be bullet-proof enough to accommodate future needs.
这样的无效代码不应该被批准，因为将来很有可能永远不使用该代码，或者该代码的防弹能力不足以适应将来的需求。
- Is it the right time to add this functionality? 现在是添加此功能的合适时机吗？
- Does the code address only the acceptance criteria and nothing more or less? 该代码仅解决验收标准，或多或少吗？
易于阅读和维护 (Easy To Read and Maintain)
Sometimes developers write super complex code for something straightforward because it makes us feel good. This is usually the case with junior or new developers.
有时，开发人员为一些简单的事情编写超复杂的代码，因为它使我们感觉良好。 初级或新开发人员通常是这种情况。
- Complexity-wise, is the code as a whole or, in part, challenging to read and understand? 从复杂性角度来说，代码是整体还是部分难于阅读和理解？
- Is the code easy to maintain and extend? 代码是否易于维护和扩展？
- Is the code easy to test? 该代码易于测试吗？
命名和文档很难 (Naming and documentation is hard)
Let’s admit it, naming is very hard and that’s why it needs the code reviewer's attention.
让我们承认，命名非常困难，这就是为什么它需要代码审阅者的注意。
The questions you should ask yourself as a code reviewer
作为代码审查者应该问自己的问题
- Do the names easily communicate what the item is or does? 名称是否易于传达项目的含义或含义？
- Are the code comments easy to understand? 代码注释易于理解吗？
- Is the code over-commented or not commented at all? 该代码是否注释过多或完全未注释？
- Is the comment necessary? 评论是否必要？
- Can a complex piece of code be refactored instead of adding comments? 能否重构一个复杂的代码而不是添加注释？
- Are the code comments meaningful and gives more information than the actual code? 代码注释是否有意义，并且比实际代码提供更多信息？
- Are all the documentation, i.e., test, build, release relating to the code change updated? 是否所有与代码更改有关的文档(即测试，构建，发布)是否都已更新？
Not all the questions might be relevant for your specific case, feel free to pick those that matter the most.
并非所有问题都与您的具体情况相关，请随时选择最重要的问题。
这是我谨记的审查代码 (Here is what I keep in mind reviewing code)
By now, I have reviewed over a thousand pull requests, and I wanted to share some of my mental checklist on how I conduct my code reviews.
到目前为止，我已经审查了上千个请求请求，并且我想分享一些有关如何进行代码审查的精神检查清单。
- Encourage smaller pull requests. 鼓励较小的拉动请求。
- Suggest other reviewers if you are not qualified enough 如果您没有足够的资格，建议其他评论者
- Ask for a pair review or walkthrough to understand the code better. 要求进行一对复习或演练以更好地理解代码。
- Plan and allocate time for code reviews 计划并分配时间进行代码审查
- Prefer reviewing the pull request locally in your favorite IDE, so you can easily navigate around changes and even run the code if need be 最好在自己喜欢的IDE中本地检查请求请求，这样您就可以轻松地浏览更改，甚至在需要时运行代码
- As you review the code take notes, these notes could be used to write a detailed code review explanation 当您查看代码注释时，这些注释可用于编写详细的代码查看说明
- Use emojis when reviewing a pull request, i.e., Use 👀 to indicate that you are reviewing the changelist (pull request), use ✅ , or 👍to approve of something, use 🥇, or 👏 to show good work. 查看拉取请求时使用表情符号，即使用👀表示您正在查看变更列表(拉取请求)，使用✅或👍批准某些内容，使用or或👏表示工作良好。
- Be courteous and empathize with the author and don’t forget that they have done the work, and you are just reviewing it, appreciate their effort. 要有礼貌并同情作者，不要忘记他们已经完成了工作，而您只是在审阅它，感谢他们的努力。
- Provide constructive feedback with explanation 提供解释性的建设性反馈
- Don’t ask silly and lazy questions that you can easily find answers to 不要问那些容易找到答案的愚蠢和懒惰的问题
- Don’t ask premature questions before you finish reviewing the rest of the changelist (pull request). Your question might find the answer to your question as you reach end of the pull request (changelist). 在您完成对变更列表的其余部分(拉动请求)的审查之前，不要提出过早的问题。 当您到达拉取请求(变更列表)的结尾时，您的问题可能会找到问题的答案。
- Flag the missing tests or drop in the code coverage 标记缺少的测试或代码覆盖范围下降
- Before you ask “Why we did this …”, take a few seconds to see if you can figure out the reason behind that change through existing code, google or other means 在您问“为什么要这样做...”之前，请花几秒钟的时间，看看是否可以通过现有代码，google或其他方式找出导致更改的原因
- Suggest better commit messages, especially helpful to junior and new developers. This is something I learned during my open-source contributions. 建议更好的提交消息，对初学者和新开发者特别有用。 这是我在开源贡献中学到的。
- Suggest squashing of unnecessary commits, keep the history of the codebase clean and easy to follow 建议压缩不必要的提交，保持代码库的历史记录整洁并易于遵循
- Don’t ask for too much, keep the scope of the ticket or task in mind 不要要求太多，请牢记票证或任务的范围
- Flag violations of coding standards and best practices and provide the link to the coding standards or similar documents in your review comments 标记违反编码标准和最佳做法的行为，并在您的评论中提供指向编码标准或类似文档的链接
- Suggest the need for adding documentation/comments to complex blocks of code or even refactoring them (as a primary alternative) 建议需要在复杂的代码块中添加文档/注释，甚至对其进行重构(作为主要替代方法)
- Comments are focused on why instead of what 评论集中于为什么而不是什么
- Look for common errors cases i.e., NullPointExceptions or Undefined 查找常见错误情况，例如NullPointExceptions或Undefined
- Look for unused imports, data structures, and functions 查找未使用的导入，数据结构和功能
- Look for indentation consistency, i.e., tabs or spaces not both 寻找缩进一致性，即制表符或空格不能同时使用
- Resolve your review comments if it is addressed 解决您的评论评论(如果已解决)
- Reference external materials and documentation to support your code review comments 参考外部材料和文档以支持您的代码审查意见
- In the event of a conflict with the author and reviewer, start a face to face discussion and if that didn’t help then bring in a tiebreaker or team discussion 如果与作者和审稿人发生冲突，请开始面对面的讨论，如果这没有帮助，请进行决胜局或团队讨论。
- Watch out for complex, over-engineered and too generic patterns 提防复杂，过度设计和过于通用的模式
结论 (Conclusion)
Thank you for reading this article, I hope that you find it helpful. I have authored a bunch more articles that you might find interesting to read.
感谢您阅读本文，希望对您有所帮助。 我撰写了许多其他文章，您可能会觉得有趣。
The following article highlights things I wish I knew earlier as a developer and you might relate with
下面的文章重点介绍了我希望作为开发人员早些时候知道的事情，并且您可能会与
I have also written an article on how to get started with contributing to open source repositories and shared some of my tips, tricks, and strategies
我还写了一篇文章，介绍如何开始为开源存储库做贡献，并分享了我的一些技巧，窍门和策略
I have written over a bunch of other articles on the following topics
我写了很多其他有关以下主题的文章
- A summary of java coding best practices Java编码最佳实践总结
- Reasons why code coverage matters and how to setup JaCoCo with Gradle and Maven-based projects 代码覆盖率如此重要的原因以及如何使用Gradle和基于Maven的项目设置JaCoCo
- Encryption and decryption using the user’s password 使用用户密码进行加密和解密
- How to Load Test 如何加载测试
Please check out my medium profile https://medium.com/@rhamedy and follow me for more future articles.
请查看我的中等资料https://medium.com/@rhamedy，并关注我以获取更多将来的文章。
翻译自: https://medium.com/swlh/how-to-do-a-good-code-review-c2cab4ef32bf
如何进行代码审查
2972
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
