“CodeReviw 很重要，我们一定要做好 CodeReview。“
相信从大家在进入公司，开始编程工作的那一天起，就会经常听到别人说过这句话。 每次事故复盘，改进措施里面也总是经常能看到：改进 CodeReview 流程，严格执行 CodeReview 。
但实际上开发时，产品需求匆匆忙忙评审，技术方案草草写个思路，代码都写不完，单元测试都来不及写（代码和人有一个能跑就行），我希望别人来给我 CodeReviw，但是我又怕他让我大改；我想帮别人做 CodeReview，但是我又抽不出时间。
可见，虽然 CodeReview 一直是我们日常开发中的重要环节，但是往往因为各种各样的原因，我们没能真正的落地实践，从而也导致在代码里面埋下了地雷。最终演变成为一边排除前人埋下的雷，一边埋新的雷，无限循环。
下面我们将会围绕 CodeReview 从 What，Why ，When，Who，How 等纬度进行讨论，希望能抛砖引玉。
1 CodeReview 是什么？
想要做好 CodeReview ，我们先了解一下它的概念：
CodeReview 是软件开发过程中的一项关键实践，旨在通过同行评审机制提高代码质量和项目可维护性。在这个过程中，开发人员将他们的代码提交给同事，后者负责检查代码中可能的错误、不一致以及不符合最佳实践的地方。
这听起来像是老师批改作业，但是实际上更像是一场友好的讨论，找出代码中不合理的地方，同时给出解决方案。在 CodeReview 的过程中，每行代码都会受到同事的仔细检视，有点像在侦探小说中寻找线索。这不仅仅是为了找错，更是一个学习和教育的机会。通过这种方式，团队成员不仅可以互相教授编程的最佳实践，还能加深对项目的理解，就像共同破解一个谜题。
2 为什么要做 CodeReview（好处）
CodeReview 能不能杜绝坏代码的上线，或者让线上不再出现 bug ？ 答案是不能！但是我们如果做好 CodeReview，能减少低级错误代码上线的概率，得到一个更安全、更高效、错误更少的代码库。争取把 Code Review 变成一种开发文化，而不是制度。
2.1 提高代码质量
这应该是最最重要的一点，通过 CodeReview，开发者可以相互指出潜在的错误和问题，从而减少 bug 的产生。这种互相监督的过程帮助确保代码的健壮性和可读性，让代码更加清晰和可维护。 很多时候我们写代码，会陷入自己的思维定式，正所谓 "不识庐山真面目，只缘身在此山中"：
- 心理盲点：编写代码的开发者往往对自己的代码产生“心理盲点”，因为他们已经对自己的逻辑、思维方式和代码结构非常熟悉。这种熟悉感导致他们可能会自动忽略那些明显的错误，因为在他们看来一切都是按照预想的方式在运行。
- 知识局限：开发者可能在某些领域缺乏足够的知识或经验，导致他们无法识别出某些潜在的问题或更优的实现方式。例如，对于某些问题或性能最优化技巧不够熟悉，可能导致代码质量受限。
- 疲劳与时间压力：长时间编程可能导致疲劳，影响判断力和注意力，使得开发者难以捕捉到错误。此外，紧迫的截止日期可能迫使开发者赶工，牺牲了细致审查代码的时间。
2.2 知识分享与团队合作
- 很多时候，互相之间的业务需求不太了解，通过查看别人的代码，可以了解实现方案，学习业务知识，学习新技术，以及不同的实现思路。三人行，必有我师焉。 通过 Code Review，我们可以高效且针对性地分享自己的知识和解决方案。在这个过程中，每个人都可以根据自己的理解和经验，提供不同的视角和方法。这不仅使我们能够获得关于代码上下文和相关知识的新见解，还可能接触到更优的实现方案。因此，Code Review 是一个互惠互利的过程，使团队成员能够相互学习，共同进步。
- CodeReview强化了团队成员之间的合作。通过共同审查代码，团队成员可以更好地理解他人的工作和思路，这有助于构建更紧密的团队协作氛围。
2.3 代码风格一致
“一千个人眼里有一千个哈姆雷特”，相同的功能，不同的人可能会有不一样的实现，并且不同的实现，都是可行的。但是确保代码的一致性是 CodeReview 中非常重要的一个方面，这对于项目的长期健康和可维护性至关重要。
- 标准化编程实践：通过定期的 CodeReview，团队可以确保每个成员都遵循相同的编程标准和规范。这包括代码格式化、命名约定、文件结构以及编程原则等。统一的编码标准有助于减少理解代码所需的时间，使新团队成员更容易上手和参与项目。
-
- 促进协作：当代码遵循一致的标准和模式时，维护工作变得更加简单。这是因为开发者可以预测代码中的模式和结构，快速定位问题所在，也更容易进行系统性的更新和改进。当团队成员都理解并遵守同样的规则时，协作变得更加顺畅。CodeReview中的交流和讨论也将更加高效，因为每个人都在相同的基础上进行讨论和建议，减少了误解和沟通成本。
3 什么时候做 CodeReview
在什么时间做 CodeReviw，是固定时间，还是代码合并前，还是发起测试验证前？这是一个值得思考的问题，但是有几个关键的时间，比较适合进行 CodeReviw。当然，也和团队开发的流程规范有关。
-
在代码合并前：这是进行 CodeReview 最常见的时机。在代码被合并到主分支之前，通过 CodeReview 确保所有的修改都符合项目的质量标准和一致性要求。这有助于捕捉任何潜在的问题，防止它们进入生产环境。
-
新功能开发完成后：当开发者完成一个功能模块或修复一个bug后，进行CodeReview可以验证实现的正确性和效率。这不仅是检查错误的好机会，也是评估设计选择和实现策略的时刻。
-
定期代码审查会议：一些团队选择定期（如每周或每两周）进行固定的CodeReview会议，不仅审查新提交的代码，也回顾现有代码。这有助于保持代码库的整洁和一致性，同时增强团队成员之间的交流。
-
关键项目节点：在项目的关键里程碑，如版本发布前，进行全面的CodeReview，确保代码库稳定且无重大缺陷。这也是验证所有功能符合项目要求和用户需求的重要时刻。
我所在的团队有固定的发布窗口，我建议是：
- 定期和事件驱动的 CodeReview：在发布窗口前至少两天对小改动进行定期代码审查，及时捕捉和解决问题，防止它们累积到关键时刻。对于较大的改动，则采用单独的评审会议，这样做可以确保复杂或重要的更改得到足够的关注和细致的审查，有助于避免后期的大规模修改。
- 技术方案的早期 Review：技术方案需要在代码实现之前进行 Review，不仅有助于在实际编码开始前就发现潜在的设计问题，还可以确保团队对技术方向和解决方案达成共识，从而减少开发过程中的误解和重工。
- 冒烟测试前的 CodeReview：针对独立的大改动，确保在进行冒烟测试之前完成 CodeReview，可以在代码进入更广泛的测试阶段之前初步验证其功能和稳定性。这样的策略有助于早期发现关键问题，避免在测试或生产环境中出现破坏性错误。
4 谁参与 CodeReview
CodeReview 的参与者可以根据项目的规模、团队的结构和组织文化有所不同，但通常包括以下几类角色：
- 开发者：即代码的编写者。在 CodeReview 过程中，原始开发者负责解释代码的逻辑和设计选择，回答审查者的问题，并根据反馈进行必要的修改。
- 同团队的审查者：通常是其他开发者，他们对提交的代码进行检查，寻找潜在的错误、提出改进建议，并评估代码是否遵守了团队的编码标准和最佳实践。同行审查者通常由一个或多个具有相关项目经验的团队成员组成。
- 质量保证（QA）工程师：虽然不是所有的 CodeReview 都会直接涉及 QA 工程师，但在一些团队中，QA 工程师参与审查可以帮助从测试的角度评估代码的健壮性和可测试性。（需求相关的 QA 参与 CodeReview 是比较常见的，但是最好在测试分析用例写完之后，要不容易陷入相同的思维定式。）
- 产品经理：在特定情况下，产品经理可能参与 CodeReview，尤其是当代码更改涉及用户界面或功能需求变更时。他们可以确保代码的实现符合产品的需求和目标。（比较少见）
5 怎么做 CodeReview（实践）
5.1 确定 CodeReview 流程
尽早确定下面的问题：
- 什么时候 CoderReview？
- 在什么环节进行？
- 是会议形式，还是共享屏幕？
- 是否需要会议主持人？
- 会议之前要不要先发代码以及文档出来，参与会议的人，提前过一下，提高会议的效率。
建议：
- 提前将 MR，技术文档，产品文档发出来，大家先心里有数，有时间先上去大致过一遍，留一些 comments 。
- 视频会议或者线下会议，时间不要太长，每次会议设立一个主持人，带领大家一起做 CodeReviw 。提交人通过共享屏幕来展示自己的改动，以及思路。（同时解答之前的 comments）
- 会议中发现问题，先记录 todo，如果不是整体思路出现大问题，不要在同一个问题过度讨论。（讨论是很爽，但是大家精力有限，review 超过一个小时会出现疲倦，最好控制在 45 分钟以内）涉及可 A 可 B 的问题，会议结束之后提供更多的证据或者想法再定夺。
- 每位参与者心态更加开放，不要认为大家在挑刺，这是一次学习的机会。
5.2 每位开发者多做一些
- 技术方案考虑更加详细：
- 在开始编写代码之前，开发者应该花时间深入分析和设计技术方案。这包括评估不同的技术选项、考虑架构兼容性、性能影响、安全问题以及维护长期性。通过创建详细的设计文档并同团队成员讨论，可以确保方案的合理性并减少后期的重构需求。
- 这一步骤也应包括对预期的挑战和风险的评估，以及备选方案的准备，确保在面对实际编码中出现的问题时，可以快速调整方向。
- 编写必要的单元测试：
- 开发者应当为所有新编写或修改的代码编写单元测试。这不仅有助于验证代码功能正确，还可以确保在未来的开发中，任何更改都不会破坏现有功能。
- 单元测试应覆盖各种边界条件和异常情况，以确保代码的健壮性。高质量的测试可以显著减少维护成本，提高代码的可信赖度。
- 写完代码，重头回去读一次代码改动点：
- 完成代码编写后，开发者应该从头到尾仔细阅读自己的代码，这个过程类似于自我CodeReview。这有助于从一个新的角度识别可能的逻辑错误、不必要的代码复杂性或任何不符合编码标准的地方。
- 这一步骤也是检查代码注释是否清晰、是否有助于他人理解代码逻辑的好机会。良好的代码注释可以显著提高代码的可读性和可维护性。
5.3 尽早完成高层方案的 Review
谁都不想自己辛辛苦苦写的代码，被别人否定了，要全部重写，还得全部重新冒烟测试，这得多崩溃。频繁的重写不仅耗费开发资源，也可能影响团队士气。 为了避免这样的情况，在编码前就对功能的整体实现方案进行讨论，是确保项目方向正确、避免重大误解和时间浪费的关键。这种讨论通常涉及开发者、产品经理、架构师甚至是用户代表，确保所有相关方对解决方案的理解和期望一致。 为了减少这种情况，重要的是在功能开发之前达成高度的共识，并在开发过程中进行定期检查和沟通，确保实现方式仍然符合预定目标和最新的项目需求。
5.4 MR 改动尽可能小
根据数据分析发现，从代码行数来看，超过 400 行的 CR，缺陷发现率会急剧下降；从 CR 速度来看，超过 500 行/小时后，Review 质量也会大大降低，一个高质量的 CR 最好控制在一个小时以内。
没有人一次愿意帮你看几千行的改动。 保持 MR（Merge Request）或 PR（Pull Request）改动尽可能小是一种广泛推荐的最佳实践。这样做有几个明显的好处：
- 易于审查：较小的改动更容易被其他团队成员理解和审查。这意味着审查者可以更快地检查代码，并更容易发现潜在的错误或问题。较大的改动可能会导致审查者难以跟踪所有的变化，从而可能遗漏关键的细节。
- 减少合并冲突：小的改动意味着更少的代码行涉及到每个MR，这减少了与其他正在进行的工作发生冲突的可能性。这种策略有助于提高团队的整体工作效率，因为它减少了解决合并冲突所需的时间和努力。
- 快速迭代：小改动允许更快的迭代和反馈循环。开发者可以更快地将改动集成到主分支中，这使得整个开发过程更加灵活和响应性强。同时，它也允许团队快速响应和解决在生产环境中发现的问题。
- 降低风险：较小的 MR 通常意味着较低的风险，因为每次改动涉及的代码和功能较少，出错的概率自然降低。这种做法有助于维护代码库的稳定性，特别是在持续集成/持续部署（CI/CD）的环境中。
5.5 合理的 Review 方向
千万不要等大厦都盖好了再去Reivew，而且当有了地基，有了框架，有了房顶，有了门窗，有了装修，的各个时候循序渐进地进行Review，这样反而会更有效率，也更有帮助。
我比较认同别人说的：先 Review 设计实现思路，然后 Review 设计模式，接着 Review 成形的骨干代码，最后 Review 完成的代码，如果程序复杂的话，需要拆成几个单元或模块分别 Review
在进行 CodeReview 时，合理的审查方向通常涵盖了代码的多个方面，以确保最终的产品质量和维护性。以下是一些关键的审查方向：
- 功能性和正确性：
- 目标：确保代码实现的功能符合需求规格说明，没有逻辑错误。
- 检查点：检查代码是否完全实现了预期的功能，是否处理了所有已定义的输入和输出条件，以及是否适当地处理了错误和异常情况。
- 可用性：
- 目标：确保代码的设计考虑到了用户的使用便利性，特别是在接口和交互设计方面。
- 检查点：对于涉及用户界面的代码，检查界面是否直观易用，用户指引和错误消息是否明确，以及用户是否能够通过简单的步骤完成任务。
- 效率：
- 目标：确保代码运行效率高，资源使用最优化，没有无谓的资源浪费。
- 检查点：分析代码中的算法和数据结构选择，检查是否存在性能瓶颈，如不必要的数据库查询、重复的数据处理或低效的循环。
- 可读性和维护性：
- 目标：代码应该易于其他开发者阅读和理解，便于未来的维护和扩展。
- 检查点：检查代码是否遵循了清晰的命名约定，是否有足够的注释，以及模块和函数是否保持了适当的大小和单一职责。
- 一致性和风格：
- 目标：确保代码遵守了团队或项目的编码标准和风格指南。
- 检查点：检查代码格式化是否一致（如缩进、空格和括号使用），命名是否规范，以及是否遵循了项目中定义的架构模式和编程实践。
- 安全性：
- 目标：确保代码没有安全漏洞，保护系统免受恶意攻击。
- 检查点：检查代码是否有潜在的安全问题，如输入数据是否经过验证，是否有SQL注入或跨站脚本（XSS）的风险，以及是否正确地管理了用户认证和授权。
5.6 开放积极的团队文化
下面是其他文章的建议，我个人比较认可： 在代码审查过程中，培养一种积极的反馈文化至关重要，因为有效的沟通并不容易实现，尤其是在提供对同事工作的反馈时。以下是一些改善代码审查中讨论质量的建议：
- 专注于代码，非个人：提供的反馈应针对代码本身，避免涉及作者个人。这有助于保持讨论的客观性和专业性。
- 明智选择讨论点：不必对每一个小问题都进行辩论。选择那些对项目有实质性影响的重要问题来讨论。
- 接受多样的解决方案：认识到对于一个问题，可能存在多种正确和有效的解决方案。保持开放的态度，欣赏不同的思路和方法。
- 团队合作精神：记住，所有参与代码审查的人都在为同一个目标努力——提升产品质量。你们是团队的一部分，共同努力才能达成目标。
- 尊重作者的感受：记住，代码的作者（除了自动依赖更新机器人，如dependabot）是有感情的人。在提供反馈时要体贴和尊重。
- 基于理由提供反馈：在提出批评时，应提供清晰的理由而非个人感受。这样的反馈更具建设性，也更易被接受。
- 采用“是的，并且……”技巧：为了保持创新氛围，避免在 Review 初期 阶段驳回新鲜而尚未成熟的想法，这可能显得不礼貌。试着建设性地添加信息，而不是仅仅否定。
- 平衡正面与建设性反馈：确保在提供改进建议的同时，也不忘赞扬作者的优点和努力。收到认可总是令人振奋。
5.7 把细节工作交给计算机
每当审阅者花时间在小细节上挑剔时，可以考虑是否可以进行自动检查，将一些细节性和重复性的工作交给计算机处理是一个高效且智能的做法。
- 自动化代码检查：
- 工具使用：利用静态代码分析工具如 ESLint、SonarQube 或 Checkstyle 等，可以自动检测代码中的语法错误、潜在的bug、代码风格问题以及安全漏洞。这些工具可以在代码提交之前或集成过程中运行，以确保代码质量符合预设标准。
- 效率提升：自动化工具可以迅速检查大量代码，比人工检查更快、更准确。这样可以释放开发者的时间，让他们专注于更复杂的问题和逻辑上的审查。
- 持续集成（CI）系统：
- 自动化测试：在持续集成环境中，每次提交都会自动运行单元测试和集成测试，这有助于及时发现和解决问题。
- 及时反馈：CI系统可以在代码合并到主分支之前提供即时反馈，帮助开发者快速识别并修复引入的问题。
- 代码格式化工具：
- 统一代码风格：使用如 Prettier、Black 等自动化代码格式化工具，可以统一代码风格，避免在Code Review中对代码格式的无谓争论。
- 减少人为错误：自动化格式化减少了人为手动调整格式可能引入的错误。
通过将细致入微的工作委托给计算机，Code Review 的过程更加专注于代码的架构设计、业务逻辑和团队协作，这样不仅提高了审查的质量，也增强了团队的生产效率。
5.8 明确每个 Reviw 结果
当我们发现一段代码有问题时，我们需要明确请求，一般有几种情况：
- 代码不正确，存在明确的 bug：
- 详细说明问题：在指出代码错误时，提供具体的错误场景和复现步骤是非常有用的。这可以帮助作者更快地理解问题并验证修复方案。
- 提供错误证据：如果可能，提供日志、错误截图、失败的测试用例或其他证据来支持您的观点。
- 要求修正：明确指出需要修正的代码部分，并建议可能的解决方案，或要求提交者提供修正方案。
- 代码可拓展性和性能优化：
- 提出建议：指出代码在性能或可拓展性方面可能存在的不足，提供具体的改进建议或更优的实现方法。
- 给出选项：提供几种改进方案，包括优缺点分析，让提交者可以选择最适合当前项目需求的方案。
- 鼓励探索新方法：鼓励提交者探索新的技术或方法，以提高代码的质量和效率。
- 不确定代码是否有问题：
- 表达顾虑：直接表达您对代码某部分的疑虑，说明觉得可能存在问题的原因。
- 咨询和讨论：邀请代码的提交者或其他团队成员参与讨论，这不仅可以澄清您的疑问，也有助于团队成员之间的知识共享。
- 保持开放态度：在这种情况下，保持开放和合作的态度特别重要，因为问题可能并不明显，需要集体智慧来解决或确认。
5.9 MR 或者 PR 信息足够清晰
当 MR（Merge Request）或 PR（Pull Request）含有详尽的背景信息和目的说明时，审查者能够更好地理解代码变更的上下文，从而做出更加准确和有用的反馈。（最好做成 MR 或者 PR 的模板来填写）
以下是一些关键的信息类型，应当在提交 MR 时提供：
- 目的和背景：
- 明确目标：说明这个MR旨在解决的问题或实现的功能。包括相关的业务或技术背景，帮助审查者理解为什么会有这样的改动。
- 关联信息：如果适用，提供与这个MR相关的需求文档、用户故事、bug报告或设计文档的链接。
- 主要改动说明：
- 变更摘要：简明扼要地列出主要的代码变更点。这包括新添加的功能、修复的bug或性能改进的具体描述。
- 关键部分突出：对于复杂的变更，指出代码中的关键部分，可能需要审查者特别关注的地方。
- 测试和验证：
- 测试覆盖：说明已执行的测试类型（如单元测试、集成测试等），以及是否有新增的测试用例覆盖新的代码。这有助于审查者了解改动的可靠性。
- 如何测试：提供测试这个MR的步骤，特别是对于需要特定配置或数据的情况。
- 影响评估：
- 影响范围：评估并描述这个MR可能对现有系统部分的影响，包括但不限于性能、安全性和用户体验。
- 回滚计划：如果变更带来问题，应有相应的回滚计划或修复策略。
- 请求特别注意：
- 特别说明：如果有任何需要审查者特别注意的事项，比如对现有功能的潜在影响，或是对某些代码逻辑的特别处理，都应在MR中明确说明。
通过这样详细的 MR 描述，可以极大地提升代码审查的效率和质量，减少来回交流的时间，确保团队成员能够快速、准确地理解和评估代码变 更。
6 常见一些代码问题
- 代码重复：发现相似或相同的代码段在多个地方被重复使用。
- 硬编码：代码中直接使用了硬编码的数值、字符串或配置，而不是通过配置文件或环境变量管理。
- 复制粘贴的代码：从其他地方复制的代码没有被适当修改或优化。
- 分布式锁的错误管理：在使用分布式锁时，锁未能正确释放，可能导致死锁或资源访问问题。
- 慢查询SQL：SQL查询效率低下，导致数据库性能问题。
- 内存泄漏：由于不当的资源管理或对象生命周期管理，导致应用消耗过多内存。
- 异常处理不当：缺乏必要的异常捕获，或异常处理过于笼统，没有适当的错误记录或处理。
- 使用不安全的API：使用存在安全漏洞的第三方库或API，或未正确验证外部输入。
- 设计原则不符：代码违反了SOLID原则，结构复杂，缺乏模块分离。
- 代码风格不一致：命名不规范，代码格式混乱，增加了阅读和维护的难度。
- 资源使用不当：不合理的全局变量使用，或未正确管理文件和网络资源。
- 并发问题：线程安全问题或死锁风险，尤其是在多线程环境中。
- 过度工程：为解决简单问题引入不必要的复杂性，如过度使用设计模式。
7 总结
《史记·礼书》有云：“不弃微末，久久为功。” code review 并非一朝一夕之功，每一个细微的改动都可能引发系统的巨大波动，故而我们在审查中总是小心翼翼，力求精确。正如破窗理论所启示的，忽视代码中的小问题可能会导致更大的问题。因此，通过细致入微的审查，我们可以及时修复这些“破窗”，避免技术债务的积累，并推动整个项目向更高标准迈进。
CodeReview 应该成为一种文化，而不是公司的一种制度，我们每一个人需要为自己写的代码负责，同时学习他人的长处：他山之石，可以攻玉。
参考：