这是一个创建于 1420 天前的主题，其中的信息可能已经有所发展或是发生改变。
以我所在的公司为例，平时项目周期已经很紧张了，根本没时间 code review 。
之前有个需求，从开发到联调再到 showcase，只给你一周的时间，这么短的时间，根本不会考虑什么 code review，只要能把功能实现就不错了，等到做完这个需求，还没等你缓过来，下一个需求就立马来了。
之前有个需求，从开发到联调再到 showcase，只给你一周的时间，这么短的时间，根本不会考虑什么 code review，只要能把功能实现就不错了，等到做完这个需求，还没等你缓过来，下一个需求就立马来了。
 |
1
x66 2021-01-04 10:03:59 +08:00  2
我司有这个流程，每次提交代码都会有两位同事和一位 team leader review 。
但是很多时候也会沦为一种形式，因为同事写的业务你并不一定都清楚，也只能帮忙看看语法有没有明显的问题。 有些同事一次改动的代码过多，很难 review， 更多的时候你自己思路来了正在 coding，同事来找你 review，很难放下自己手里的工作去仔细看他的代码 |
 |
2
ferock 2021-01-04 10:14:24 +08:00 1
楼上+1
参考鹅厂，有 code review，svn 递交必须审批，回顾问题落实到责任制。 那可能沦为形式化会弱一点。 制度和人之间，关键作用的还是人。 |
 |
3
Mirage09 2021-01-04 10:17:36 +08:00 via iPhone
看情况吧，小的 CR 走个形式，大的 CR 还是比较严肃的
|
 |
4
NexTooo 2021-01-04 10:17:37 +08:00 6
因为安排了 code review 的任务，也不会给你 review 的时间……
|
 |
5
duduaba 2021-01-04 10:19:18 +08:00
code review 最大的阻塞就是很大的 commit，如果各个功能改动细分 commit，那 review 就很容易了。
|
 |
6
jzmws 2021-01-04 10:26:45 +08:00
小公司根本没办法做到 CR ,有时候看看别人的代码最多看看代码是否符合规范，注释啥子是否描述正确，其他的真的不好再弄
|
 |
7
jdhao 2021-01-04 10:29:07 +08:00
能跑就行。
|
 |
8
gggxxxx 2021-01-04 10:30:24 +08:00 2
code review 只在 beta 时期有用。软件大部分都完工了，这个时候任何的代码修改，大家一起开会 review 看看，看看会不会因为个人修改代码考虑太少造成其他部分额外的风险。
我一直很不认同互相 review 代码风格啊，具体实现啊。毫无意义的事情。一方面，别人看你代码可能根本看不懂，每个人负责的模块和业务都不一样，草草看看也不实际测试，然后提交一个已阅标记........给自己一种感觉仿佛好像自己是美帝互联网大厂一员..... 另一方面代码风格其实根本不重要，只要能正确运行的代码就是好代码。我曾见过真正美帝互联网巨头公司的员工代码，每个人都是风格各异性格突出，有些人就喜欢 tab 空 2 个 space 呢..... |
 |
9
xuboying 2021-01-04 10:37:39 +08:00
@gggxxxx #8 代码风格不是项目一开始就定好了么，机器检查，项目风格都定不下来不是技术负责人压不住小弟么。要么就是没有负责人。。。
|
 |
10
chiu 2021-01-04 10:37:44 +08:00 1
merge 代码的流程就有 review (自己 review/verify -> 机器人 review+机器人 build+机器人 test -> leader/owner review), 以上整个流程通过后, change 才会自动 merge 到对应 branch 上
|
 |
11
saulshao 2021-01-04 10:40:33 +08:00 3
代码 review 的核心目标其实是尝试让其他人理解作者的代码。
挑刺永远都比做事容易，如果各位理解这点，就会明白 code review 其实是非常重要的，应该要切实执行。 |
|
12
gggxxxx 2021-01-04 10:51:01 +08:00
@xuboying 为啥老是开口就是权力话语这套说辞呢....压不住小弟....喷了。
一个团队里的成员有自己的个性其实是好事，在不重要的事情上做各种约束其实意义不大。想想看，团队里每个人都穿格子衬衫开发效率就会提高么？ |
 |
14
Lemeng 2021-01-04 10:55:18 +08:00
没有绝对的，重视程度有大不同
|
 |
15
ericcode 2021-01-04 10:55:25 +08:00
来这家公司 3 年了，一直坚持做 review，很有用，但是真的很费时间
|
 |
16
b1ackjack 2021-01-04 10:56:43 +08:00
之前小公司 sonar 和人工 cr 都有，来了这二线啥都没了
|
 |
17
hantsy 2021-01-04 10:58:45 +08:00
@chiu 这个流程差不多可以。
我现在只要参与的项目 100%需要 CR 。主要创建的 PR 在进行。 进行 Code Review 之前，必须保证（这些都是由 CI 完成）： 1，所有代码通过测试 2，测试 Coverage 报告（指标认可， 测试不到位，添加测试） 3，代码质量检测工具报告达标（如果引入新的 Bad Smell 代码等，需要重构清除）（现在智能化的检测工具，借助大数据，通过海量分析，可以检测很多代码质量的问题，及预测运行时产生的动态问题，不仅仅是命名规范等的问题） 这之后才是人工 Code Review，加入一些修正意见 4，Code Review，进行重构 还是重申我的观点，不写测试，做 CR，CI 都会成为形式主义。 |
 |
18
tkl 2021-01-04 11:07:40 +08:00
1L + 10L
sonarqube |
 |
19
channingcheng 2021-01-04 11:19:59 +08:00
@saulshao 说的太对了， 遇到一个总挑别人的人，什么都要按照他的想法来实现，是时候真不想搭理这人
|
|
20
channingcheng 2021-01-04 11:21:36 +08:00
@hantsy 有个问题，cr 后重构，那测试不就要重新测了？确认改那么多东西没有问题吗？
|
 |
21
timedivision 2021-01-04 11:25:41 +08:00 1
@gggxxxx 代码风格不定好，拉代码合并的时候，你 2 个 space，他一个 tab，你咋合并？难道自己不用格式化代码的吗
|
 |
22
Orenoid 2021-01-04 11:33:54 +08:00
我司是有 CR 的，小型创业公司。
虽然谈不上形式化，但这个流程目前基本上已经变成，以 “应该按我的想法来实现” 为目的了。反正我现在也懒得争论了，对方怎么说我就怎么改……说到底还是话语权的问题，总该有个人让步。 |
 |
23
taowen 2021-01-04 11:45:31 +08:00
code review 只是一种给反馈的手段而已。如果能尽早提出改进意见，没必要等到 code review 的时候来做。cr 时候提一堆意见去返工，浪费也挺大的。
|
 |
24
weer0026 2021-01-04 11:47:41 +08:00
小公司表示只有在出现性能问题才会去 code review，新员工刚来也会看看，感觉大部分小公司都这样。
|
 |
25
IsaacYoung 2021-01-04 11:47:47 +08:00
走形式
|
 |
26
mazyi 2021-01-04 11:59:33 +08:00 via iPhone
垃圾人做辣鸡事，都是垃圾业务怎么来厉害的技术呢？
|
 |
27
Tonni 2021-01-04 12:10:39 +08:00
我们公司会，Code Review + CI + Unit Testing，就是产品做的一塌糊涂
 |
|
28
saulshao 2021-01-04 12:20:21 +08:00
@channingcheng
19# 我想表达的愿意其实是写代码的都不喜欢被人 Review，而参与 code review 的人都应该会喜欢。 所以逻辑上应该所有开发都参与 code review，这其实不是看有多少正面效果，而是增强互相之间的理解和沟通。 |
|
29
hantsy 2021-01-04 12:26:43 +08:00
@channingcheng 你从来没用过 CI 吗？？？
|
|
30
hantsy 2021-01-04 12:28:16 +08:00 1
@saulshao 这一点没错。更多应该 Peer to Peer Code Review，建立有效的沟通机制，而不是什么上级 Review 下面的人的代码。
|
 |
31
Cstone 2021-01-04 12:42:56 +08:00
以前有，几个开发大家约个会议室一起认真评审代码，后来来了个领导，说减少工作时间开大会私下指定人 review 就行，代码评审也就成了走过场，再到现在没人 review 了
|
 |
32
leega0 2021-01-04 12:50:56 +08:00
小公司就如你所说，开发周期根本没有 review 的时间给你，基本匆匆上线，各种改 bug 再测，循环往复。
|
 |
33
dayeye2006199 2021-01-04 12:51:16 +08:00 4
我们公司只有三个人，也有 code review，有 CICD，有测试覆盖需求。这是一个企业文化的问题，对这点我们不妥协。
|
 |
34
lights 2021-01-04 13:00:25 +08:00
刚毕业在传统 IT 行业的时候，没有 CR
后来跳槽做了两年多的互联网，配合 gitlab 做 CR 感觉很棒，偶尔能学到新姿势，也有利于增强代码维护性 再后来转行做游戏，虽然都是用 SVN，但第一家公司有做简陋的 CR，就是看一遍再用 QQ 或者口头和你沟通 现在依旧做游戏，没有 CR，虽然公司小，但毕竟有不同的人做同一个模块，偶尔会觉得比较蛋疼 |
|
35
hantsy 2021-01-04 13:16:14 +08:00
@dayeye2006199 这点没错，不用 CI，不写测试，所有 CR 都会是形式。
那种开会形式讨论，什么盯着纯代码提意见的，我也经历过，没任何实质性的作用。 具体 CR 实施一定要有的放矢，那么在 CI 中运行生成的测试报告，质量报告就是那个“的”，连一个最基本的讨论<<基点>>都没有的话，只能说大家公司都是在走过场，说白了，你们根本就不是在 CR 。 |
|
36
channingcheng 2021-01-04 14:22:07 +08:00
@saulshao 可惜我们这边都是只有业务 owner 才 code review，大头兵没有资格 code review
|
|
37
channingcheng 2021-01-04 14:37:57 +08:00
@hantsy CI 和重构是两个问题呀
|
|
38
hantsy 2021-01-04 14:38:10 +08:00
@channingcheng 你们那不是 CR 。只是普通的抽查工作情况（恰好拿代码挑刺说事），跟以前学校检查宿舍卫生一样，只有学工处干的事。
|
|
39
hantsy 2021-01-04 14:43:39 +08:00
@channingcheng 不知道怎么跟你解释你的问题。。。
|
 |
40
Perry 2021-01-04 14:44:59 +08:00 via iPhone
Unit Testing, Code Review, Integration Testing, Accessibility Testing, Stress Testing, Chaos Testing, Load Testing 等等这些不通过估计进不了大公司的 Production 。
|
 |
41
llllboy 2021-01-04 15:09:53 +08:00
就是个摆设
|
 |
42
DiverRD 2021-01-04 15:26:47 +08:00
我一个版本改了 70 多个文件 组长看了 直接放弃 review
|
 |
44
USAA 2021-01-04 17:19:33 +08:00
code review ？ 这玩意不应该自己来弄吗
|
 |
45
flowerains 2021-01-04 17:30:38 +08:00
有时间才能互相 code review
比如我们公司如果是压着点做需求，能按时按量把需求作完上线就不错了 |
|
46
hantsy 2021-01-04 17:55:04 +08:00
@DiverRD 说明你们项目管理问题非常大。一个 PR 一般只包含一个 Feature，不应该太多，正常一般几个文件吧。如果是 Bug，可能可能几行代码，几个字符。
|
|
47
hantsy 2021-01-04 17:57:04 +08:00
|
 |
49
onec 2021-01-04 18:52:02 +08:00
纯纯的摆设
|
 |
50
dfzj 2021-01-04 20:08:54 +08:00
在中型公司，项目分三个等级，只有 P1 等级才会 code review 。也就是是被定义为核心基础依赖的项目。
一般的业务项目，功能测试过了就发布了。 |
 |
51
irytu 2021-01-04 20:30:14 +08:00
我们公司 code review 很多都喜欢鸡蛋挑骨头 要说意义么 几乎可以忽略不计
|
 |
53
pangleon 2021-01-04 22:06:50 +08:00
楼上很多人没理解一点，CODE REVIEW 真正受益的是说人，他在给你介绍自己代码的时候也是重新梳理自己实现逻辑的流程，这个过程很能帮助他提高自己的思维甚至发现 BUG
|
 |
54
akira 2021-01-04 22:24:16 +08:00
不做 cr 的产品 确实是能跑能上线，但是后面出问题你要花费数以倍记的时间。 单元测试也好，压力测试也好，代码评审也好，这些都是为了降低风险引入的，做了后续出问题的可能性小，不做后续出问题的可能性大。
出来混 迟早是要还的。 |
 |
55
yexiaoxing 2021-01-04 22:26:14 +08:00 via iPhone
我们合规要求必须有人 review 才能 checkin 然后发 release 。
|
 |
56
lagoon 2021-01-04 23:12:54 +08:00
我觉得，先是设计好，然后开发好，再然后才是测试好。
Code Review 肯定是好的，也是有用的。 但许多中小公司的情况是，设计没办法好，开发没办法好，然后领导指望抓测试，指望 Code Review，就能稳住质量。 本末倒置，只是安慰剂。 能不能设计好，取决于领导。能不能开发好，取决于领导。 领导不从自己身上找原因，寄希望于给开发人员找茬来稳住质量。 因此，大部分公司，不会 Code Review，或者只能进行走形式的 Code Review 。 |
 |
57
hugo54 2021-01-04 23:34:48 +08:00
我所在的产品线的后端，都是 QA 来 CR 。 凡是非自主测试的代码，保准给你一行行 review 。
|
 |
58
vagranth 2021-01-05 00:54:37 +08:00 via Android
我司不光有 review，而且还要求写测试 case 。
我曾经有一个大 patch 被要求拆分成 5 个 patch 慢慢 review 完的经历，要不是那个功能架构做的还可以，拆都拆不开。 |
 |
59
msg7086 2021-01-05 01:23:34 +08:00 via Android
我们是强制 code review，除了其他开发以外，team leader 也要 review 一次，没问题了才能合并。
|
 |
60
Leee 2021-01-05 08:32:30 +08:00 via Android
@dayeye2006199 你们在广州吗？还要人吗😂
|
 |
61
cwliang 2021-01-05 09:21:04 +08:00
code review 很有必要，可以防止一些 anti-pattern 、性能冲击、维护性，虽然没时间看具体业务逻辑
|
 |
62
jorneyr 2021-01-05 09:29:47 +08:00
理想很丰满，现实很骨感
|
 |
64
leekafai 2021-01-05 10:13:00 +08:00
sr
self review |
 |
65
runliuv 2021-01-05 10:49:38 +08:00
有个锤子
|
 |
66
LemonK 2021-01-05 12:42:01 +08:00
@timedivision lint 和风格是两码事。比如有个前同事：他用他自己发明的标准划分为同类的变量，要求别人必须统一前后缀命名；多种语法都 ok 的逻辑，要求必须按他喜欢的那种写法；别人写 if 的地方他要求改成三目，别人写三目的地方他又要理论一下 if 更好。如果之前在这种代码孔乙己的身边工作，强调一下代码风格不重要还是有必要的。
|
|
67
timedivision 2021-01-05 13:51:04 +08:00
@LemonK 风格不是一个人制定的，不管怎么说，代码这种二进制的东西，实现某个功能逻辑有很多种方法，但一定是有一个最优解的，尽管说简单的 if 判断扯不上什么最优解，但对于项目本身来说，统一的风格一定是利于维护的
|
|
68
LemonK 2021-01-05 15:00:01 +08:00
@timedivision “写法一定有一个最优解”这句前提完全不认同。听起来就好像天下同一题目的作文只有一种标准写法一样。除了“一定不要这样写”的 Bad smell 共识之外，自由发挥的空间依然非常大。除非你只写 JAVA 。
|
|
69
timedivision 2021-01-05 15:57:30 +08:00
@LemonK 不要断章取义啊，我说的是实现某个功能逻辑的方法一定有一个最优解，另外你拿作文和代码相比，这两者根本没有可比性
|
 |
70
mlbjay 2021-01-05 17:06:25 +08:00
项目组有个老同事，技术很强，但是 CR 后 总我“建议”我改一些“可有可无”的东西，我也不敢反对，改咯。
|
Select Language