一、为什么要code review
需要有Code review环节,流程规范
为什么要Code Review?
希望能提高代码质量,提高团队技术,减少bug。
二、code review常用流程规则
比较常用的有:Git Flow+Pull Request(PR)
Pull Request(PR) 是你没有权限往一个特定的仓库或分支提交代码,你请求有权限的人把你提交的代码从你的仓库或分支合并到指定的仓库或分支。
由于PR需要有权限的人确认,所以适合在这个过程中做Code Review。
在支持PR模式的软件里,每一个PR都有一个新增代码的对比(diff)界面。
代码审核者可以在线浏览请求合并的新增代码,并针对代码行添加评论。
评论可以被所有有权限查看仓库的人看到,每个人都可以回复任何人的评论,有点像论坛里某个帖子的讨论。
这种模式是事后审核,也就是代码已经提交到了中心仓库,Review过程中频繁的改动会造成历史签入记录的混乱。
由于Git太灵活了,因此诞生了很多的Git流程,用来规范Git的使用。
常见的有集中式工作流、功能分支工作流、Gitflow工作流、Forking工作流、Github工作流。
我们对Git Flow做了些调整,调整后的流程被命名为Baza Flow,定义见后文。
根据Baza Flow,我们大部分仓库只定义了2个主干分支,master和develop。(例外,我们有一个仓库有3个开发小组同时进行开发,定义了4个主干分支,目前还比较顺畅,再多估计主干分支之间的合并就比较繁琐了。)
master对应生产环境代码,所有面向生产环境的发布来源都是master分支的代码。develop则对应本地测试环境的代码。
绝大多数情况下,QA(测试)只测试develop分支和master分支的代码。
由于开发人员都在一个团队内,所以我们没有采用基于仓库的PR,采用的是基于分支的PR。
我们对主干分支的操作权限做了限制,只有特定的人才能操作,develop分支是项目开发Leader和架构师,master分支是QA。
有权限往主干分支合并的成员会按照约定的规则来执行合并,不会合并没有完成审核的PR。
上面这点其实蛮重要的,所以我们会对有权限合并的人有特别的约定,在什么情况下才能合并代码。(见后文PR的说明)
PR的发起人要主动的推动PR的审核,Leader也会密切关注PR审核的进度,在需要的时候及时介入。
我们配置了CI服务器(什么是CI)只编译特定的分支,通常是develop和master分支。
所有的代码合并到了主干分支之后,都会自动触发编译和本地测试环境的发布,QA无需依赖开发人员编译的代码来测试,也无需自己手工操作这些,保证了开发人员和测试人员的相互独立。
我们本地测试环境的发布包含了数据库和站点的发布,全自动的,发布完成以后就是一个可用的产品,有时间这部分也可以分享一下。
我们还使用了Scrum里面一个很重要的概念:完成定义。
就是我们规定了我们一个任务的完成被定义为:代码编写完成,经过自测,提交的PR经过审核并且合并到主干分支。
也就是说,所有的代码被合并到了主干分支之后任务才算是完成,而被合并到主干分支必须要经过Code Review,这是强制的。
三、谷歌的code review
谷歌code review开源地址: https://github.com/google/eng-practices
中文相关:https://xindoo.github.io/eng-practices-cn/review/reviewer/
https://xindoo.github.io/eng-practices-cn/review/developer/
审核者指南
开发者应当不断改进 ,否则代码库整体质量也不会提升。
审查者的责任是让代码库越来越健康,但是要有度,不能要求太苛刻。
代码对系统有明显的提升且正常工作,即便不完美,评审者也应该倾向于通过这次变更。
评审者应该追求 持续提高 ,而不是追求完美。
原则
- 技术和数据高于意见和个人偏好。
- 关于风格问题, 风格指南是绝对的权威。任何不在指南中指出的样式(比如空格等)都是个人偏好的问题。
- 软件设计是有时候也会有多种有效的选项,如果开发者能证明(通过数据或者原理)这些方法都同样有效,那么评审者应该接受作者的偏好,否则应该遵从软件设计标准。
- 如果没有其他的规则使用,只要保证不会影响系统的健康度,评审者可以要求开发者保持和现有的代码库一致。
解决代码冲突
如果Code Review中有任何冲突,当很难达成一致时,开发者和评审者不应该在Code Review评论里解决冲突,而是应该召开面对面会议或者找个权威的人来协商。
如果这样都解决不了问题,那解决问题的方式就应该升级了。通常的方式是拉着团队一起讨论、让团队主管来权衡、参考代码维护者的意见,或者让管理层来决定。不要因为开发者和评审者不能达成一致而把变更一直放在那里。
亮点
如果你看到变更中做的好的地方,告诉开发者做的很棒,尤其是当他们用某种很精巧的方式解决你评论中提到的问题时更应不吝赞美。Code Review过多关注于错误,但你也应该为开发者展现出好的一面点赞。
礼貌尊重开发人员
给予指导
接受解释
缩写解释:
开发者指南
一个CL描述是做了什么更改以及为什么的公开记录。
要言简意赅。
不要掺杂个人情感
如果评审者批评了你的代码,可以理解为他们在帮你、帮整个代码库、甚至是帮整个公司,而不是攻击你或者是质疑你的能力。
修复代码
添加注释来解释清楚代码的逻辑。如果评论似乎毫无意义,那么您的答复应该只是代码查看工具中的解释。
自我反思