这是一个创建于 2294 天前的主题，其中的信息可能已经有所发展或是发生改变。
我现在所经历的公司，还没有做 CodeReview 的，也许公司太小的原因，但是我觉得 CodeReview 是非常有效的方式，能提高代码可控性和质量，还能促进技术交流。不知道你们经历的公司有 CodeReview 么，有的话可以分享一下感受，和如何进行 CodeReview 比较好。
101 条回复 • 2018-08-19 12:33:36 +08:00
 |
1
anonymous0user 2018-08-14 11:10:17 +08:00
小公司比大公司更容易 CodeReview ……
|
 |
2
peterswan OP @anonymous0user 为啥我经历的小公司也没有 CodeReview，感觉有的公司是因为项目赶，有的根本就没有这个意识...
|
 |
3
liprais 2018-08-14 11:12:16 +08:00 via iPhone  1
做数据库的，不做 code review 就是找死
|
 |
5
NCry 2018-08-14 11:29:47 +08:00
我们公司就两个开发。。。
|
 |
8
yuanrenxue 2018-08-14 11:42:34 +08:00
@NCry 一个写代码，一个补漏 哇咔咔
|
 |
9
zclHIT 2018-08-14 11:42:57 +08:00
做啊，每个版本快结束的时候改 pmd, checkstyle, codedex, simian 改到想吐，不过代码冲刺的时候 CodeReviewer 就直接点 accept，从来不看。。。
|
 |
10
enenaaa 2018-08-14 11:45:38 +08:00
现在的这家不但做交叉 review，入库还要审批。 重构优化代码有时候还找不到理由发申请。
|
 |
11
xiaket 2018-08-14 11:46:00 +08:00
找人过 PR 好烦... - .-
|
 |
12
kaito 2018-08-14 11:47:42 +08:00
在公司推行 Code Review 的话需要看看周围同事是不是有那种意愿
如果是平级推进的话，看看同事的技术和代码风格是否 ok，可以的话找他讨论一下有没有意愿，有些程序员很无所谓的，觉得写「好代码」太浪费时间了，review 就是 挑刺的过程，自尊心太强的会非常抵触。 推行这些的前提需要比较好的同事关系和工作素质，如果都不错的话可以尝试推进一下 |
|
13
peterswan OP @zclHIT 我觉得不错，对于代码质量和自己习惯都有提高，还可以对于优化进行讨论，不过代码冲刺的时候我觉得可以预留这部分时间，太急的情况只能例外了。
|
 |
14
hasbug 2018-08-14 11:48:54 +08:00
上一家我去的时候给我说会，我害怕我的面条式代码会有危险，待了一段，没有的事。。。
|
|
15
peterswan OP @kaito 恩恩，我觉得可以和同事商量一下，对于一些代码风格问题可以统一定一个规范，Review 过程也是提高的过程。
|
 |
19
crayygy 2018-08-14 11:57:26 +08:00 via iPhone
我们 team 里面是必须过的，而且至少有两个人，一个是需要是本模块的，另一个可以是其他的，而且 arch manager 都会查最近的 commit 有没有按照规定来完成
|
 |
20
mikuazusa 2018-08-14 12:02:10 +08:00
持续集成流程强制必须过 CR
|
 |
21
winterfell30 2018-08-14 12:02:41 +08:00 via Android
提代码的时候必须要有该模块的负责人打分，但是感觉大家也不怎么看随手就给过了
|
|
22
zclHIT 2018-08-14 12:02:43 +08:00 via iPhone
@peterswan 每次都能把一个月的需求压缩到一周然后搞冲刺，扯皮 2 周，冲刺 1 周，测 1 周
|
 |
24
poorcai 2018-08-14 12:37:23 +08:00 via iPhone
我们部门每周一次
|
 |
25
klren0312 2018-08-14 12:43:56 +08:00 via Android
讲真的，做外包本来就很敢，恨不得早点写完，没人愿意看代码
|
 |
26
d18 2018-08-14 13:01:10 +08:00
大公司也有代码质量很差的，只要最后能跑起来就行，比如鹅厂。
|
 |
27
wobushizhangsan 2018-08-14 13:03:12 +08:00 via Android
今年做了个项目别说 review，连测试都没了。开发，上线，生产问题，补丁。
|
 |
28
wuzhizhan 2018-08-14 13:07:53 +08:00
今天需求，明天上线
|
 |
29
dangluren 2018-08-14 13:14:08 +08:00
哇，和楼主很像，目前的公司也不做，之前经历过的都做，虽然有时候有的自尊会受到打击，但感觉是非常有效的方式，由于不做，目前的项目是天天出问题。向领导反馈过，领导貌似自己都不想做
|
|
30
peterswan OP @dangluren 恩恩 这个就像 12 楼所说的，要大部分人都有这个意识，尤其是技术领导或者团队领导，否则会趋于形式化起不到作用，现在我们也没有这个意识，大多数人更在乎速度，对于质量都想着以后优化，但是以后永远在以后。
|
|
31
peterswan OP @d18。。。没去过大公司还，不过我觉得大公司推行这个应该更容易吧，资源充足，技术大牛也多，不过这个还是要看领导了
|
 |
32
monkeylyf 2018-08-14 13:59:07 +08:00
长远来看不做 code review 就是自杀行为。
code review |
|
33
monkeylyf 2018-08-14 14:01:26 +08:00
从中长远来没有 code review 看就是自杀行为。
了解同事的技术水平，了解同事在做什么，肉眼抓 bug，等等等。 因为要急着上线不审代码的先不说技术水平如何， 工程水平肯定有提高空间。 |
 |
34
HuHui 2018-08-14 14:31:19 +08:00
回武汉后就没做过了。
|
 |
35
hiluluke 2018-08-14 14:49:32 +08:00
先发 issue,然后技术讨论，然后再发 pr。pr 再 review 一遍。PR 过大，需要拆分。。。
|
 |
36
imdupeng 2018-08-14 15:10:47 +08:00
没有 codereview，不过老大总喜欢改别人的代码，每次他的意见都很中肯，但是你改了要调试好啊，每次改了出问题，还得我去调试好。
一方面催进度，一方面跑来改我代码找麻烦。。能不能等逻辑跑完了再来优化呀？搞得我反复在原来的代码上做工。 |
 |
37
mhtt 2018-08-14 15:13:24 +08:00
其实对没有足够的 code review 经验，而去做 code review 是件挺难受的事情
|
 |
38
coolhubery 2018-08-14 15:17:03 +08:00 4
我部门是做数据库执行引擎的，C++ 14。CodeReview 到了“变态”的地步。。。
所有 change 都必须尽量的小，必须写 Unit Test，代码覆盖率 100%。一个 change 从开始 push 到最终 merge 最少怎么着也得 5 到 6 轮，当然国外的大牛同事基本上 2/3 个 patch 就 merge 了。 从错别字，注释的准确性，英语表达是否合适，代码可重用，设计角度是否合适，性能是否有影响，Unit Test 是否真得测到了该有的功能，是否应用了 C++ 14 的标准等诸多方面进行考量。。。 这样做的好处是作为非常核心的一个组件，模块非常稳定，新增代码量很大，bug 却非常少。 虽然作为个人来讲，初期自己的修改进展很慢，要被 challenge 很多次，但是一旦熟悉了也就很快了。 不管是那个公司，核心的组件必然要经过极其严格的 Review，否则后期的成本会非常高。 |
 |
39
nicevar 2018-08-14 15:17:49 +08:00
配置 SonarQube，减少 code review 工作量，同是提升代码质量，还能让人的强迫症变得更严重
|
 |
40
Just1n 2018-08-14 15:18:01 +08:00
我们每个 checkin 都必须有 code review。
而且，如果修改了不属于自己组的代码，会自动触发一个流程让其他组的人介入进行 review。 |
 |
42
czk1997 2018-08-14 15:29:37 +08:00
能跑就行，出 Bug 再说……测试全靠脸（能跑就行），服务器部署全随缘..
|
|
44
peterswan OP @imdupeng 下次不要让他直接该代码，让他提 pr 或者开个分支，做好了之后你来 Review，有问题让他改好了在合并，不过这好像是你的老大。。。
|
|
45
peterswan OP @coolhubery 这个流程有很多值得学习的地方啊，能提供这样严格的 Review 肯定公司也特别棒了，谢谢分享
|
 |
46
lsyAndroid 2018-08-14 15:36:32 +08:00 via Android
没有，全靠自觉，本身就是赶项目的，没有时间搞
|
|
47
peterswan OP @nicevar 用自动化工具是会加快这个过程，不过我感觉每次 pr 应该粒度尽可能的小，Code Review 压力就不会那么大了，而且可控性更高，如果太大的 pr，直接让他拆分后再 Review。
|
|
48
peterswan OP @Just1n 恩恩 强制的 Review 才能保证代码可控，不过我觉得为什么要让其他组的人来 review 呢，毕竟其他组的肯定不熟悉业务，找同组的人会不会更节省时间。
|
 |
49
66beta 2018-08-14 15:45:44 +08:00 via Android
本组的人，没人负责一个模块，也只能 review 语言本身了，无法察觉业务漏洞
|
|
50
peterswan OP |
 |
51
A555 2018-08-14 15:46:42 +08:00
形式大于内容
|
|
53
peterswan OP @66beta 对于语言和代码优化相关的 Review 也会提升代码质量，更关键能在 Review 过程中提升自己，既可以挑别人代码的不足也可以学习别人代码的优点。
|
 |
55
natscat 2018-08-14 15:54:08 +08:00
做啊 代码功能 实现方式 都做
|
|
56
monkeylyf 2018-08-14 16:03:48 +08:00
@peterswan 我亲身经历过没有代码审核，快糙猛急着上线，等代码库到一定体量一个没更新一个版本都是提心吊胆，开发人员身心俱疲，最后项目烂了，公司黄了。
话说回来，我现在所待的地方，是有代码审核的，同样老板喜欢催上线，大佬们都相互给个 LGTM 都不仔细看。现在代码体量已经很大了，基本两天一个中性 bug 爆炸，一周一个大 bug 核爆。 流程是用来约束自觉性的，但是完全没有自觉性，有再多流程也没用。 |
 |
57
loveCoding 2018-08-14 18:11:56 +08:00 1
组内在使用 gerrit 做强制 code review 还算比较好用
|
|
58
peterswan OP @monkeylyf 恩恩，这个要看有没有一个厉害的技术管理去领导做这个 Review 事情，对 Review 质量进行把关，如果只是应付形式简直就是浪费时间了，还有就是对于 Code Review 可以有一定的奖励制度调用积极性。
|
 |
59
RorschachZZZ 2018-08-14 18:44:10 +08:00
@monkeylyf 中等或者严重的 bug，你们那不复盘下、讨论下吗
|
 |
60
zhaoxinz 2018-08-14 19:31:48 +08:00
在就职过的公司中，只有印象笔记有融入在日常工作中的标准流程 Codereview，在 commit 之后，通过命令行指定一个人为你 Codereview
|
 |
61
hoichallenger 2018-08-14 20:38:37 +08:00
我们公司有 Code Review, 不过正打算用 Pair Programming + Mob Programming 取代 Code Review
|
|
62
monkeylyf 2018-08-14 20:48:39 +08:00
@RorschachZZZ 讨论的。但都是基于表面或者 bug 本身的讨论，深层次原因应该大家都知道但是不会拿出来说罢了。
|
|
64
peterswan OP @hoichallenger 刚刚了解了 Pair Programming 和 Mob Programming，真是个不错的想法，公司的技术管理也很不错啊。
|
|
65
peterswan OP @loveCoding 准备搭建一发推荐给团队用。。。
|
 |
66
agagega 2018-08-14 21:18:10 +08:00 via iPhone
你们在 review 时有些特别特别小的改动也要新增提交吗？有时候想 commit amend 然后 force push，但是又觉得不太好。看来还是 git 太自由了，用 hg 就没有这种强迫症。
|
 |
67
fanqianger 2018-08-14 21:23:00 +08:00
@coolhubery 你们什么公司。
|
|
68
peterswan OP @agagega 应该是按照功能细分粒度的，我觉得尽可能小的好。还有远程提交信息不是不应该修改嘛，除非只有自己提交分支。而且我觉得应该过了 Review 再入库。
|
 |
69
nanyang24 2018-08-14 21:48:22 +08:00
很幸运的是，所在的公司技术 leader 强制了这项规定，合并代码前必须经过任意两个同事的 CodeReview。
我觉得好处是项目代码健壮性得到了一定的保证，对个人的话是学习认识不同风格和 level 的代码，还能熟悉别人做的需求，对整体项目理解有帮助。 |
 |
70
qiaobeier 2018-08-14 22:00:45 +08:00
@coolhubery sap 核心部门的啊，运气真不错，我前同事分去做业务据说非常废人。。。
|
|
72
coolhubery 2018-08-14 23:49:32 +08:00
@fanqianger SAP...
|
|
73
coolhubery 2018-08-14 23:50:46 +08:00
@qiaobeier 不了解产品，SAP 的数据库平台技术还是相当不错的。
|
|
75
fanqianger 2018-08-15 04:33:44 +08:00
@coolhubery 这厉害的啊，hana 挺牛逼的。你是在德国吗
|
 |
76
liyuhang 2018-08-15 07:02:56 +08:00
我觉得应该先 Review 一下你的标题
|
 |
77
jiangjz 2018-08-15 07:51:25 +08:00 via Android
不做，所以有的代码维护起来让人想离职😂😂
|
 |
78
jaaazzz 2018-08-15 08:51:42 +08:00
中等规模公司，做个屁
|
 |
79
micean 2018-08-15 08:53:47 +08:00
作为管理人员想知道你们有多少时间花在 code review 上，我每天都忙得要死
现在只能了解下面人的代码水平，交流一些好的习惯，从编码规范上要求他们自觉 |
 |
80
dosmlp 2018-08-15 08:55:52 +08:00
没。。。。。。时。。。。。。间。。。。。。
|
 |
81
lxerxa 2018-08-15 08:57:42 +08:00
团队小的时候还做，后来慢慢放弃了。。。
|
 |
82
wuhanchu 2018-08-15 09:00:39 +08:00 via Android
有 review 都是好公司啊
|
 |
85
shijingshijing 2018-08-15 09:17:49 +08:00 via iPhone
@coolhubery 如果养成习惯了就真不错，大家共同进步，code review 还有一些计划类的 paperwork 其实从长远看都是必不可少的。如果为了赶工期某一个人开始写 smelly code，而且不加制止就 merge 进来，最终导致的是整个 project 散发臭味。就跟加班一样。
ps：比较好奇你们的覆盖率达到 100%，是指需求覆盖率还是语句覆盖率？如果是严格按照 MCDC 做语句覆盖，每个条件都 cover 到，那 100%就厉害了。 |
 |
87
UIXX 2018-08-15 09:20:48 +08:00
有 CodeReview 要两点：
1、领导强制力 2、员工自觉性 团队要有能人，员工要会进取、肯学习。其他都是扯淡。 |
|
88
coolhubery 2018-08-15 09:28:36 +08:00
@shijingshijing 语句 100%覆盖。除了极少部分遗留代码无法实现 100%的情况外，新增代码必须语句 100%，否则 Gerrit CheckBot 静态检查都过不了。
|
|
89
coolhubery 2018-08-15 09:29:19 +08:00
@fanqianger 西安...
|
|
90
peterswan OP @micean 就我的经历来看，靠自觉解决不了工程中出现 smelly code，只要有这种代码，就很有可能会让项目代码维护成本增高，而且如果维护的人写的代码继续糟糕下去，会使的项目走向慢性死亡，这些时间还不如拿出来放到开发的时候去 Review，其实是节省的整体项目的时间，管理人员更应该有这个意识。
|
|
91
shijingshijing 2018-08-15 09:43:41 +08:00 via iPhone
@coolhubery 那这个成本很高了，我只在某些 safty critical 的软件上看到过类似甚至更严的方法，比如 pacemaker 产品不仅对覆盖率有要求，而且对实时性有非常高的要求，必须在指定时间内完成指定任务。
你们测试是自己做还是外包给三哥？包括 test plan，test case，test report 都是怎么做的？ |
 |
92
Rico 2018-08-15 09:45:51 +08:00
有。且代码评审作为升等的一个参数。
|
 |
93
thinkmore 2018-08-15 09:57:54 +08:00
每次提交代码都要 code review.
好处：学习别人写的好的代码，让别人指出自己写得不合理的地方或者造成歧义的地方。 坏处：codereview 有时候时间太长，无法快速合并代码影响开发效率。 |
|
94
coolhubery 2018-08-15 10:00:54 +08:00
@shijingshijing SAP HANA 内存数据库执行引擎。
因为是全新的执行引擎去替代老的引擎，所以大部分的端到端测试，性能测试，压力测试等都可以重用。自己的 change 提交的话是需要自己写 unit test 的，一些本引擎特有的特性需要自己额外写 E2E 测试。 |
 |
95
xoxo419 2018-08-15 10:07:28 +08:00
人少 活多 事赶
|
 |
96
wujiyu115 2018-08-15 10:09:36 +08:00
阶段性 review,一个开发周期结束,上线之前 review 一下
|
|
98
micean 2018-08-15 12:38:25 +08:00
@peterswan 本来也是半路接手的项目，项目本身的设计就有问题，从去年开始就跟上头不止一次提过要重构整个设计，以免项目越来越膨胀的时候彻底改不动了。然而一点卵用都没有，1 版本还在测试，2 版本的需求就过来了，3 版本的需求就开始规划了，设计思想基本没有，有点无力回天的感觉了
|
|
99
peterswan OP @micean 对于这种项目，上头的人如果只想着眼前开发的速度，不考虑可维护性，如果你本身也没这么大的权利去实施重构和审查，我觉得这个项目已经走向凉凉的道路了。如果能够实施，复盘重构是必须的了，重构加入代码审查，人力充足的话老版本的迭代和重构版本同步进行，员工心里写代码也会开心提高效率。
|
 |
100
mingyun 2018-08-18 22:21:01 +08:00
那么问题来了，有什么好的 code review 工具
|
Select Language