我发现 我公司的 review code 这个目的已经变质了。

[kpppp]

我呆过几家公司，都是千人以上的规模。但是 review code 给我的统一感觉就是 [官僚] 。

明显的语法错误项目组 leader 也看不出来，只是看一眼，这些有+2 权限的人，大多数都没有做过你的项目，只是级别高。在入库这一关，卡死你，每次都要发消息，告知对方，请帮我 review ！

比如我们最近的一个政策： 领导说:为了提高我们代码质量，所以我们要一起 review 。就是让写 c 或 c++的人和写 java 或 kotlin 的人互相 review code 。
实行了 2 天了，普通工程师说的最多的几句话:
1.看不懂，不知道你写的啥。
2.效率好低啊，没啥鸟用。

高级与专家工程师说的最多的几句话:
1.你这个代码有点冗余啊
2.这个没有问题吧
3.ok ，我马上+1

首席与经理说的话：
1.ok
2.已经+2

于是我直接把整个项目组的人都加入了 review，然后引起了 leader 的不满,说我太自我了.于是我又一个一个的 remove reviewer 。

哎，难啊。

[feather12315]

@lecher #20 新手代码都搞不懂，咋写实现细节？

[wobuhuicode]

新入职的公司也是这样…… review 和 没 review 一个样，都是为了现实上级有干活的发个 已阅。
上线之后依旧一堆 bug

[zpxshl]

说说我司情况
1 cr 不需要领导＋ 2，事实上一般也不会找领导 cr
2 改动到特定模块强制需要模块负责人 cr
3 代码不符合语法规范的代码提交时自动拦住

总体还行，毕竟模块负责人也不希望你的改动影响到他的模块。至于自己模块写的代码找人 cr，主要看对方是否负责，负责的会慢慢看仔细看，同时我也会在旁边讲解。 不负责的直接就＋ 2 了。 呆几个月你也知道谁靠谱谁不靠谱了。

当然了，cr 者其实是无偿帮你，用爱发电了，所以找人 cr 时态度好点，大的改动尽量分批 cr，多点注释，画个流程图就更好了。一来就上千行业务代码的提交，反正我是没心情看得。。。

[0Shaka]

比较好的实践就是平时只找组里一两个人 review, 高效快速还没有那么多 b 事.

[ga6840]

个人认为公司里的代码也应该有 maintainer，而且他和他维护的代码肯定是利益相关。你的 PR 肯定应该是这些人 review 。无关的或者平级的再或者高出几层的领导来看的话最多好心看看。如果组织没有什么奖励热心的制度，那么八成也就没有人有意愿。

[linux40]

聘几个搞程序分析、语义分析的吧。

[lecher]

@feather12315 特别新的新手挑几个写好细节设计的文档让他写实现，尽可能调新增业务或者特别简单的 bugfix

[MrGba2z]

CR 这件事情我觉得从效率提升上来说的优先级是:
1. 好的 infrastructure: 我们公司代码的自动检查不仅仅负责语法, 规范, 还会自动优化 条件表达式 (比如一堆 XOR 如果可能的话他会改成更短更合理的形式) 以及 best practice 和 deprecated 的自动修复. 最后会跑配置好的测试. 这些全通过之后才会发送 CR 请求, reviewers 都会收到推送提示 (macOS 的话就是系统通知, 也有 Chrome 插件会持久监视)
2. Reviewer 人员的选择, 每个人 review 代码的严格程度是不一样, 如果你想快一点, 就选已知的比较松的人, 如果想提高或者对自己写的代码没信心, 就找 review 严格的人.
3. 组里 review 代码的风格. 我们 TL 之前介绍过他的风格, 不一定适合所有人但是我觉得挺不错, 主要的几点是: 1. 如果代码达到了目的, LGTM. 2. 可读性 > 语言规范 3. 小 CR > 大 CR 但是如果 大 CR 已经写好了, 除非自己看不懂, 否则尽量不要要求对方拆分. 除了这个之外, 我们组基本上收到 CR 会在 5~30 分钟内完成 Review. (没人要求这么做, 只是大家都这样, unblock 别人也是间接提升整个组的效率)
4. 就是自己写的代码了, CR 描述是否能提供足够的背景? CR 是否太大而不方便 Review? (比如我看到个 2000 行的 CR, 本能的会想 这 TM 得找个 30 分钟的时间段慢慢看, 现在没空 马上要吃饭了, 然后就.....)

当然话说回来,很多事情不是自己能改变的. 如果你是 TL 倒是可以尝试改变. 如果有机会的话, 去大公司去哪怕实习一个月, 也能看到别人的这套流程有多好.

[DamienS]

这些问题不难解决吧
积极性:
1. 代码 review 加入到季度员工 kpi
2. review kpi 包含质量考核

规范：
1. 规范 UI PR 要截图，规范日志要写啥
2. 规定一些谁可以接，几个人接了就可以（一般一个人接了就行吧，不然影响效率，而且写 c++的 review java 代码这种就很扯淡，经理接也很扯淡）
3. 规定所有的评论下个版本就要全改完，不能改一半，别人又指出有的还没改

简化工程师负担：
1. 每个 PR 自动化监测一些基本错误，比如用 linter，或者端对端测试，没过的回去改，直接不到 review 那一步
2. 建立一些 review group 。不用一个一个加或者减

[laike9m]

这 review 就离谱。还是多学学正规的大公司是怎么做 review 的吧

[DamienS]

@MrGba2z CR..是亚麻老哥么

[meetubyaccident]

代码没注释，随便看看就行，TM 费时间还要猜你为啥你这么写，很累人的，自己的活不用干吗？出错自己改。

[yaaaaaak]

语法问题 ci 都过不去吧

[leavic]

都一样，做硬件十几年，早就知道了一个基本定律：所有人都不靠谱，不要指望别人帮你检查出任何错误。

[hpeng]

那不是。那些都叫代码合并工程师。

[b00tyhunt3r]

@wuzhouhui
老哥 想问一下详细的提交日志具体是指啥，git commit 绑定到编辑器吗

[hantsy]

@kpppp 只能说你公司的实施问题吧。

可以看看这两位的观点。
@lecher https://www.v2ex.com/t/725245#r_9776838
@huifer https://www.v2ex.com/t/725245#r_9776373

在编码前，对于一起讨论需求的理解，最终确定代码要完成哪些功能，在开发之前也要归纳出来，可以在 PR 或者 Issue 形成一个 Checklist，例如一个表单注册。
1 。 如果信息填写不完整，Validation 失败，提示错误
2 。 信息完整，提交后，检测 Email，手机号是否存在，如果存在，反馈错误信息
3 。 如果一切检测通过，保存用户信息，（发送 Welcome 邮件，初始一些用户配置等）

首先，对于现代软件开发，特别云环境，一开始就需要一个自动化的 Pipeline 配置，检测分支代码情况，一目了然。
https://github.com/hantsy/spring-reactive-jwt-sample/pull/59
1，Commentlint 等，
2，检测语法（比如老式的静态检测 Checkstyle, findbugs 等如果你喜欢可以配置下，甚至可以加入到 Git Hooks 里面。不过基于云的 SonarCloud 检测更详细，我不再用本地静态分析工具）
3，运行测试（单元测试，功能 /集成测试）<--------必不可少， 如果不写测试，CR 只能是自欺欺人。
4, 生成测试报告，包含代码质量报告（ SonarCloud 的规则是 Social 更新的，可以支持最新的语法，规范等。公司如果大方，可以购买 Code Climate， 借助人工智能分析代码，可以修正弱智代码），可以看到 Bugs，Code Smell，技术债务等

那 CR 的时候，我们要清理所有在这个 PR 产生的技术债务，修正 Bad Code Smell 等，而且要检测功能集成测试是否包含了所有的路径，是否有漏掉什么步骤等。

CR 几乎不需要 [人工方式] 关心一些枝节细节，什么语法，命名等，这些工具检测（在 CI ）都可以完成。更多的目的大家一些讨论和分享，真正做到 Pair programming 和 Brainstorming 。

[andj4cn]

我觉得跟开发团队层级管理有关。开发团队应尽可能扁平化，我一般代码只被 2 个人 review，最多了。其他人想 review 也可以

[ClericPy]

哈哈哈 其实挺羡慕有正经 code review 的公司的, 现在天天赶进度文档都跟不上, 别说 review 了

PS: 听说有一种设计就是让不懂这门语言的人也能看懂你代码的, 类似于注释言简意赅地做到类似小黄鸭的程度

[charlie21]

哈哈 “业务 review” 比 “code review” 更重要。光 review code 没啥意思