Code Review 指导方针
优质博文:IT-BLOG-CN
Why Code Review? - 为什么要进行代码评审?
Code Review是软件开发过程中的一个关键实践,它有以下几个重要目的:
Improve Code Quality- 改进代码质量
【1】确保代码符合团队的编码标准、最佳实践和设计原则。
【2】识别并修正可能影响长期维护的问题,如过于复杂的逻辑、硬编码或缺乏注释等。
【3】消除代码的Bad Smell,避免后续提交者踩坑。
Early Detection of Code Flaws - 及早发现设计上的缺陷
Code Review 可以帮助识别代码中的错误、缺陷和潜在性能问题,在发布到生产前进行修复。同时也是对业务逻辑和系统架构进行Review的过程。
Team Collaboration - 促进团队协作
Code Review 可以促进团队成员之间的协作和沟通。通过讨论设计决策、实现细节和改进建议,团队成员可以增进彼此的了解,对代码风格和架构设计形成共识。
Reduce Code Ownership Silos - 减少代码所有权的集中性
【1】Code Review 有助于减少代码所有权的集中,防止业务知识和责任集中在个别团队成员身上。避免了意外来临时某个应用没有备份人员的情况。
【2】通过评审过程,多个团队成员可以熟悉代码库的不同部分,从而提高总体的代码评审覆盖率和最终的可维护性。
Code Review Process - 代码评审流程
评审步骤
一个完整的Code Review流程应该包括以下步骤:
1、作者完成功能改动,创建到主分支的Merge Request,并根据团队的 MR Template 编写MR描述信息。
2、配置的CICD Stream中的自动化工具(Sonar,Lint,UT Coverage)执行初始代码静态分析和检查
3、AI Code Assistant自动为代码提供反馈和建议
4、作者处理自动化工具扫描出的issue,Critial必须更改,Major新代码必须更改,旧代码尽量更改,其余级别根据实际情况根据代码规范酌情修改。
5、作者处理AI Assistant评审意见并回复,更新MR
6、作者分配MR给人工 Reviewer
7、Reviewer阅读MR Template中的描述信息,理清代码改动背景与具体改动点,有必要时单独和作者沟通。
8、Reviewer根据不同维度(下一章节Review Dimensions中提到的)来检查代码,并提供反馈。
9、作者处理Reviewer提出的issue,根据意见完善MR Template描述,或更改代码。
10、作者重新提交代码,重新开启评审流程。
11、Reviewer批准更改,代码完成CICD流程后合并到主分支。
流程图
For Reviewer: Efficient Code Review - 如何高效进行代码评审
Pick a Reviewer - 选择评审者
【1】评审者应该是能够对你的更改了解最为全面的人。他们不一定是代码库的Owner。所以有时候我们需要让不同的人来review同一个更改,例如一位负责review业务逻辑是否正确实现,一位负责review代码规范和风格是否满足代码库要求。
【2】评审者应该优先选择有一定工作年限,review经验较为丰富的团队成员。他们可以用自己的工作经验帮助你挖掘出代码中潜在的缺陷。
【3】为了培养团队成员的审查技能,可以采取互相审查的方式。高级工程师审查初级工程师的代码,也可以请初级工程师一起审查他们的代码,这有利于知识传递和成长。某些重要变更,也可以通过交叉审查来降低潜在的风险。
Review Dimensions - 评审的维度
Code Reviewers 可以根据以下几个方面去对代码做审查,注意:
【1】要确保Code Review中的每一行代码都有被检查到,错误往往会藏在意想不到的最容易被人忽略的改动中。
【2】需要同时查看代码改动的上下文,局部正确不一定代表全局正确。
【3】确保代码库的健康状况没有恶化。
Design - 设计
Code Review 中最重点被检查的部分应该是变更的整体设计,包括:
1、代码的输入输出,方法和模块的交互是否合理?
2、变更中新增的代码是否可以和系统的其他部分很好的集成?是否会和别的模块产生冲突?
3、变更的代码的应用范围是公共组件还是业务代码专属?其他开发人员能否在其他场景调用?
Functionality - 功能
1、新增或改动的功能是否符合需求,产品经理,开发人员的原始意图?
2、该功能对是否符合用户的预期?是否会在某种程度上给用户造成不便或者操作上的复杂性?
3、在极端情况下,该功能能否按照预期的工作?非理想环境下(例如弱网环境,连接中断,外部接口鼓掌)是否会为业务带来损失?
4、如果功能会直接影响终端的操作或展示(Offline页面或对客App),Reviewer应该要求提供功能演示,或者预期行为的演示。
5、涉及到金额(公司损失)和用户敏感信息(可能的PR事件)的代码需要尤其谨慎,Review中应该特别关照。
Complexity - 复杂度
1、为了实现需求的功能,该代码的实现还能更简单吗?是否比应有的复杂度更高?
2、Reviewer是否可以快速理解代码的实现?其他开发人员将来遇到此代码时是否能够轻松理解和使用此代码?
3、是否存在过度设计导致简单的功能变得过于复杂?Reviewers需要做好可拓展性和过度设计之间的定义和平衡。
4、是否符合单一职责原则?一个方法或者一个类是否做了过多的事情导致它过于复杂?
Tests - 测试
1、代码是否对每个功能都有Unit Test?整体的行覆盖率和分支覆盖率是否达标?
2、Unit Test中的用例是否清晰明确?每个测试是否都有Assertions?
3、测试中是否包含该功能的边界场景?例如用户输入了不符合预期的超长字符,或者不合规范的手机号,测试会失败吗?
4、除了单元测试,该变更是否已经完成集成测试和自动化测试?用例是否完整?通过率是否达标?
Naming - 命名
1、变更的代码中,类名,方法名,变量名,他们的命名是否都合理且清晰,能够解释他们本身的用途?
2、命名是否符合订后约定的格式和规范?例如MQ Topic,包名等。
3、命名是否可以在保证清晰含义的前提简化?例如UserInfoUpdateHandler.handleUserInfoUpdate就是没有必要的。
Comments & Documents - 注释 & 文档
1、注释的格式是否符合代码规范?例如多行注释,变量上的注释,和代码行上的注释使用的格式是不一样的。
2、Reviewer是否能够在不求助作者的前提下看懂每一行注释?
3、所有的注释是否都是有必要的?
4、注释是不是在解释代码存在的原因而不是代码本身在做什么?如果需要注释来解释代码本身在做什么,则应该考虑代码本身是否过于复杂,是否能够简化。
5、对外提供的契约,公共API是否都有明确的文档?文档中是否包含了目的、如何使用以及使用时的行为?
6、功能和流程发生了改变后,文档是否有对应的更新?
Style - 风格
1、代码更改是否符合订后的代码风格指南?(链接待补充)
2、如果Reviewer觉得某个代码样式可以改进,但是不是强制性的,可以使用“Nit:”(Nitpicking)标注
3、如果代码不符合代码风格指南或规范,但与周围代码一致,鼓励作者添加TODO在后续改进
Encouragement - 鼓励
1、Code Review不应该只关注错误,如果有好代码也应该提出赞赏和鼓励,尤其是对经验不足的新员工。
2、告诉开发人员他做对了什么有时候比单纯指出他们的错误更有价值,更值得他们思考。
Guidelines for Writing Comments - 提出评审意见的指南
Always be kind - 礼貌,和善
1、评论需要礼貌,尊重代码作者
2、评论只可以针对代码,不能够评论代码作者
3、避免攻击性语气,提出问题而不是直接提出要求,同时避免反问句。
4、Reviewers要谦虚,你不永远是对的,不要夸张,不要讽刺和刻薄。
Explain your reason - 解释你的原因
1、提出意见时,需要解释为什么要这样做,可以引用数据,生产实践例子,规范(例如阿里代码规范,订后code style规范某一条的截图)。
2、Reviewer应该为作者提供适当的帮助,但是需要在指出问题和直接指导之间取得平衡
3、要给作者留下思考的空间,因为作者通常会比Reviewer更加接近代码
Label comment severity - 根据重要性分级
当提供非强制的意见时,根据重要程度使用标签进行区分:
NIT: 从风格上我觉得你这么写更好,但是不做也不会有什么影响
Optional: 我觉得这样写更好,但是不是严格要求的
FYI: 本次变更中可以不修改,但是我期望未来的变更中可以修正这个问题
For Author: Best practice for submitting Commit & MR - 提交与合并的最佳实践
对代码作者来说,在提交Commits和提交Merge Request的时候应该遵循本最佳实践,可以提高Code Review的效率,并减少和Reviewer的协作和沟通成本。
Small and Simple Changes
1、每次Commit/MR都应该是独立的变更,但要尽量小,只解决一件事,只包含一个需求/技改的一个完整的功能模块
2、每次提交的MR都应该包括完整的UT cases并都要通过(如果是紧急发布的场景可以例外)
3、合理的大小: 一般一次MR不 应该超过1000行或超过20个文件
4、如果一次开发涉及到的代码超过了合理大小,则需要对变更进行拆分,成为功能相互独立的增量式的MR。
5、每次提交小型MR的好处:
- Reviewer可以更快速,更彻底的进行Review。与其占用他一个小时,不如多次占用他10分钟。
- 对Reviewer来说,小块的逻辑更容易被理解,更容易发现其中的隐藏问题。
- 出现问题时,回滚方便,返工的工作量也会变少。
Writing good MR descriptions
MR描述是你对工程所做变更的一个公开记录,他传递了非常重要的信息,无论是Code Review还是后续进行迭代管理和需求调研都非常重要。一个合格的描述应该至少包括以下的内容
改动了什么? 对你更改的一个总结,Reviewer可以不用看完整个diff就可以知道你改了什么东西
为什么要改动? 需求/技改的背景,实现中的考虑
索引: 改动对应的需求编号/技改编号
该描述将成为我们git历史版本的永久组成部份,未来的开发人员会根据你的描述来搜索MR,试图弄清楚“当时做了什么,为什么这么做”,以决定后续的功能迭代中不会出差错。一份好的MR描述不仅对团队的工作和代码质量有重要作用,也是对自己的工作的一个review的过程。
MR description的编写应该遵照团队制定的template模版:
主要包括了以下组成部份:
- Description: 简要阐述进行了什么改动
- Reference:相关链接,如JIRA,Conf等等
- Implementation Details: 为什么要改动?改动的背景,考虑,设计和重要的实现细节。
- How Has This Been Tested:
如何测试你的改动?列出一些测试case - Checklist: 确保在合并进主分支之前正确的实施了其中的每一项操作
Reviewer Mindset - 评审者心态
Ensure code quality of every MR.
1、Reviewer 有责任确保每个MR的质量,以确保代码库的整体代码质量和架构健康状况不会随着时间的推移而下降。
2、在实践中这可能比较棘手,因为代码库通常会随着时间的推移和多样化的功能需求而退化,尤其是当团队面临严重的交付时间限制,没有时间进行良好的调研和设计评审,只能走捷径才能实现目标时。
Take ownership of the whole projects.
Reviewer 需要对他们所审阅代码的整个工程项目质量负责,需要保证新增的代码风格和现存代码尽量一致,并且后续是可以维护的。
Any improvement is good.
1、如果MR中有部分代码超出了需求范围,但可以改善线上系统的整体代码健康状况(例如在不影响功能的情况下优化了代码结构,或提升了原有代码的性能),即使这部分改动并不完美,Review 也要倾向于批准和鼓励这些改动。
2、没有所谓的“完美”代码,只有更好的代码。Reviewer需要在交付效率和更改的重要性之间取得平衡,持续改进,而不是强行追求完美。
Technical facts overrules personal preferences.
1、在风格问题上,团队的编码规范具有绝对的权威,规范中未提及的点都算是个人喜好问题,优先保持和工程中其余代码风格一致,否则,除非可以显著增加可读性或可维护性,请尝试接受作者的代码风格。
2、代码结构和功能架构的设计应该基于软件设计的基本原则进行权衡,而不是基于个人喜好或者拍脑袋。如果作者可以通过线上数据或者特殊情况说明证明他的设计同样有效,则Reviewer应该接受作者的处理方式,否则,则取决于软件设计的基本原则和团队中的代码和架构规范。
More discussion when conflicts.
当Reviewer意见和作者冲突时,更多的沟通和讨论是必要的:
1、开发者和评审者应首先尝试根据代码规范,设计原则,工程实践达成共识。
2、如果很难达成共识,面对面沟通或视频会议可能更有助于解决冲突。
3、如果仍无法解决,可以采取上报措施,如扩大讨论范围到整个团队、征求技术leader意见。
4、不要因为无法达成一致而长时间搁置MR。
Author Mindset - 代码作者心态
Write good change descriptions and code comments.
1、代码变更的清晰描述和重要代码的注释非常重要,它传递了更改了什么和为什么要进行更改两个信息。
2、阅读源代码只能说明程序做了什么,而无法得知为什么要这么做,这么做的背景是什么,导致后续维护变得异常困难。
3、未来的代码维护者会使用模糊的描述性词语查找你的变更,如果没有清晰的描述或注释,找到你变更的相关信息会非常困难。
4、清晰的代码注释和变更描述可以节省评审者的时间,是对评审者的尊重。
Learn from reviewers.
1、对评审者的反馈和建议持开放态度。记住,代码评审是学习和改进的机会。
2、评审者是在评判代码,而不是你这个人。不要把反馈当成是针对你个人的。
3、积极接受与评审者合作的机会,从他们的经验和观点中学习。即使评审者不一定是正确的,你也能从讨论中获得should or should not的思考过程,从而反射出代码潜在的改进机会。
Take responsibility for your code.
1、请对你写的每一行代码负责,忽略代码质量可能导致后续的灾难。
2、在提交MR之前,确保已经进行了充分的测试和自我评审。
3、当评审者提出issue时,好好解释你的设计决策和权衡,积极solve threads。
4、在处理完所有issue后,需要通知评审者重新评审。保持积极主动的沟通。
原文地址:https://blog.csdn.net/zhengzhaoyang122/article/details/143841018
免责声明:本站文章内容转载自网络资源,如本站内容侵犯了原著者的合法权益,可联系本站删除。更多内容请关注自学内容网(zxcms.com)!