
code review 好处很多,可以规范代码、传递知识和保证代码质量等,但是因为项目进度和其他因素,不一定保证 review 被有效执行。请教各路大神都是怎么做的?有没有啥套路、工具、方法之类的。
1 zhenjiachen 2016-07-11 10:11:11 +08:00 技术经理:什么是 code review ? |
2 bigtan 2016-07-11 10:12:11 +08:00 完全不需要,自己写,自己用。 |
3 chousb 2016-07-11 10:13:20 +08:00 One on One Code Review. |
4 lution 2016-07-11 10:13:45 +08:00 不同的组不一样吧,反正我们组是必须 review 的。公司层面应该到了一定规模的公司很难规定这个东西。 |
5 Yc1992 2016-07-11 10:13:55 +08:00 方便某个人离职后其他人迅速交接 |
6 tabris17 2016-07-11 10:14:51 +08:00 review by myself 功能都来不及赶, review 个毛线 |
7 repus911 2016-07-11 10:16:32 +08:00 自己搭了个 gitlab 上线开 mr 有权限合的人去 review |
10 mrwangrj 2016-07-11 10:21:29 +08:00 review 邮件 是展示工作量的方式。。。 |
11 zzzzzy OP @zhenjiachen 哈哈 |
12 knightdf 2016-07-11 10:22:19 +08:00 自写自测自 review 自上线 |
15 murmur 2016-07-11 10:35:53 +08:00 每个新人 的第一个模块要 review 的 或者新系统的 每个人的 第一个模块 后面看造化了 |
17 hxtheone 2016-07-11 10:37:29 +08:00 我们公司是所有的代码都要 review |
18 zhouxuchen 2016-07-11 10:46:36 +08:00 via iPhone 刚进公司的时候负责人大概 review 了一个礼拜我的代码(当然到底有没有 review 我就不知道了)。后来一切就看一个人的命运和历史的进程了( |
19 zvving 2016-07-11 10:59:07 +08:00 大家对 code reivew 好不重视啊。 我们是用 gitlab - merge request 做的, develop 分支不能直接提交代码,全走 MR ,自己不能 merge 自己发的 MR 。 好处就不说了。 |
20 Phariel 2016-07-11 11:00:37 +08:00 via iPhone Atlassian 的 Stash ,还有 Gerrit 都很好用 |
21 aitaii 2016-07-11 11:09:44 +08:00 每周 4 下午有一个 code review 确实能规范公司的代码风格 |
22 jmc891205 2016-07-11 11:10:34 +08:00 我们用 code collaborator 不过别人憋了两礼拜的代码一口气传上来让 review 的时候。。真的觉得不想去看 |
23 quericy 2016-07-11 11:14:31 +08:00 产品比开发都多,还来得及 review? |
24 sudoz 2016-07-11 11:21:56 +08:00 极个别的组有 review ,真的很少会审查代码,加班都来不及马丹的 |
25 clino 2016-07-11 11:24:20 +08:00 我们这里不自觉做 code review,做不好 code review 的工程师设计质量和代码质量都比较差 |
26 rekulas 2016-07-11 11:28:22 +08:00 500 强外企 用的 gerrit 来 view view 要经手好多人 最后的审核者还在国外流程有点慢 |
27 rekulas 2016-07-11 11:29:29 +08:00 而且审核的也很严格 比如行尾多了一个空格。。。是过不了的 |
29 odirus 2016-07-11 11:33:52 +08:00 加班赶进度都来不及,还啥子 view 哦, just make it work 公司人力资源多、钱多、注重技术,一般都会。 |
30 menc 2016-07-11 11:37:59 +08:00 one on one ,用 gerrit 来做 review 。 说不 review 的,都是没 review 过的, review 一下就知道了,注释, coding style 还有 bug ,问题茫茫多。 代码不仅是写给自己看的,在公司里也是写给别人看的, review 不止有挑 bug ,也有保证自己代码别人能读懂的关系在。 code review 不仅会找出 bug ,也会要求修改格式,添加注释以增加代码可读性。 在 V2 上看到有人推崇不写注释,要求代码自解释,来一个稍微大点的团队就知道,这根本就是狗屎。 |
31 imswing 2016-07-11 11:39:07 +08:00 技术经理:什么是 code review ? |
32 DevineRapier 2016-07-11 11:47:03 +08:00 不知道什么才算 review ,我们这写好了提交个 diff ,过了就可以提交。 review 是,先提交,过后在一起回顾审核? 感觉我们是 preview... |
33 cnly1987 2016-07-11 11:49:09 +08:00 via iPhone 人多 就需要 review ,不然你敢 merge 其他人的分支吗 |
34 expkzb 2016-07-11 11:56:19 +08:00 @DevineRapier 提交 diff 在 phabricator 里就算 review 。感觉这样挺好的 |
35 monkeylyf 2016-07-11 11:59:25 +08:00 没有 最后代码库简直臭气熏天 |
36 4everLoveU 2016-07-11 12:00:49 +08:00 RD 提测, QAreview |
37 nullp 2016-07-11 12:01:20 +08:00 全靠自觉,,接收人骂爹 |
38 hemingway 2016-07-11 12:05:05 +08:00 via iPhone 有 |
39 gefranks 2016-07-11 12:19:02 +08:00 据说是有 review 的,也有 reviewer 的名字,但是仍然不工作或者 bug 百出 |
40 hahastudio 2016-07-11 12:31:22 +08:00 |
41 kkzxak47 2016-07-11 12:37:36 +08:00 via Android 我们组加上负责人才 3 个人,名存实亡 |
42 sc3263 2016-07-11 12:39:23 +08:00 之前的项目组有。 svn 上新建 work 目录,上传原始代码,上传正式代码 ->更新 redmine 上对应的案件,通知相关人员 review ->review 代码,有啥问题提一下,有啥 bug 修复一下,都在 redmine 上更新 ->结束了上传到 trunk 目录,结束 redmine 上的案件 代码质量确实高,不管是代码风格、注释,还是业务逻辑。写的人可能只考虑当前模块当前需求, review 的人会考虑到系统整体。毕竟 reviewer 对整个系统的了解,比我们不知道高到哪里去了。至于耗时什么的,我是觉得,整套流程跑起来之后,其实也不会多花很多时间,毕竟这边在 review 的时候,那边可以继续做别的需求的开发。 |
43 qqzj 2016-07-11 12:41:34 +08:00 有,即使我改一行代码都有两个人来 review ,你得解释清楚为什么这么改,他们看得非常认真。有时还不止看改动,还把前面的代码也看下。顺便说下,他们都是工作 2 , 30 年的人,有时我都不好意思。 |
44 czheo 2016-07-11 13:01:27 +08:00 有,两人 review 之后才能 merge 到 master branch |
45 yangxiongwei 2016-07-11 13:10:46 +08:00 用 phabricator ,强制 review |
46 easing 2016-07-11 13:11:13 +08:00 必须要有啊, code review 对质量的保证还是很有效果的 |
47 SourceMan 2016-07-11 13:12:58 +08:00 看造化吧,新功能都不够时间做 |
48 shenxian 2016-07-11 13:17:31 +08:00 木有 |
50 kimmykuang 2016-07-11 14:24:32 +08:00 gitlab 会发 commit 邮件,然后我偶尔会 review 下我们组的提交;每个项目版本提测前会组内 review |
51 broadliyn 2016-07-11 15:43:47 +08:00 每次看到这类问题,我就想到那张图。 要 copy - paste , good design 又不会给你加工资。 人生没有时间给你重构。 review 有个蛋用,功能都赶不及。 逃 |
52 young 2016-07-11 15:44:55 +08:00 都是形式 review 个蛋蛋 |
53 zzzvvvxxxd 2016-07-11 15:48:54 +08:00 要的,你可以前期自己 review 后期让实现功能的小鲜肉串讲一下,介绍一下代码的组织和设计的思路,你听一听就好了。 |
54 Ixizi 2016-07-11 16:04:54 +08:00 每次 commit 必须 review 。 所有的队友必须一起 review 。 |
55 Tonni 2016-07-11 16:08:51 +08:00 我们公司用的 GitHub Enterprise ,没个代码改动都要发送一个 Pull Request ,没个 Pull Request 必须要至少两个人 review 并且给予 后才能 merge 到 master 里面去。 |
56 daochouwangu 2016-07-11 16:13:05 +08:00 用 facebook 的 phabricator 来做 code review |
57 messense 2016-07-11 16:42:45 +08:00 Bitbucket PR + 自行开发的 CI 自动化测试、 Code Lint 自动化代码风格检查 |
58 lovedebug 2016-07-11 17:05:15 +08:00 每次提交修改前都要 review 吧~~ 公司用的 perforce, 和 Idea 以及 JIRA 结合 review code 还不错 |
60 flydogs 2016-07-11 17:07:44 +08:00 有 review 率,有指摘率 |
61 iphantom 2016-07-11 17:28:10 +08:00 用的 git 每次都需要 review 然后再合并 我感觉 leader 只看看大概逻辑和写代码风格 特别是有无性能特低的代码,有没有良好的缩进和注释习惯 |
62 NovemberEleven 2016-07-11 17:28:30 +08:00 外包行业,做项目的时间都不够,唉,还 review 。 |
64 tianlang1989 2016-07-11 17:48:44 +08:00 有,个人觉得很重要 我们公司时间也很紧,天天晚上加班 然而 CR 流程一直保留 |
65 clorts 2016-07-11 18:13:43 +08:00 @Tonni github enterprise 价格多少? @Ixizi review 代码要看多久? @broadliyn review 是闲的蛋疼公司才干的么?功能都赶不及, review 个蛋啊:) @yangxiongwei 用这个多久了?是不是很烦躁呢? @repus911 review 这么麻烦啊?是一行行代码看过去么?都能看懂么? |
66 lightening 2016-07-11 18:15:02 +08:00 有,必须有人 review 才能 merge 。 |
67 repus911 2016-07-11 18:29:14 +08:00 @clorts gitlab 我们 gitlab 用开源版 还没有特殊需求 有的大功能比较麻烦,数据库看一遍, redis 看一遍,核心逻辑看一遍,兼容性过一遍,确认一遍。中间还有打回去修改,那边一个 sprint 10 天开发过来的 mr 怎么也得对上个 1 、 2 个小时。 当然这种情况比较少见,一般 bugfix 10 分钟,小功能半个小时搞定,还有熟练工发过来的来基本也是 10 分钟内。 别说我还真遇见过单元测试和 QA 没覆盖到的低级语法错误,嘿嘿嘿。 |
68 yangxiongwei 2016-07-11 18:40:05 +08:00 |
71 robert9484 2016-07-11 18:52:47 +08:00 via iPhone 没有啊 |
72 DingSoung 2016-07-11 19:21:56 +08:00 新功能没有。旧的代码,涉及底层公共的一般我自己写,让小弟看看久合了。剩下的就让测试妹子把关了。 |
73 ourcubk 2016-07-11 20:19:53 +08:00 代码->gerrit->自动编译 /UT/复杂度校验->review->review 通过触发自动编译生成 img->自动生成包->自动安装包->重启机器->启动完成后触发各种 case 跑自动化测试->代码 gerrit merge into git brunch->git brunch 自动 merge into brunch->终于他妈妈的完成了........................................................................................................... review 都是小事,重要的耗时间的都是 review 无关,却严格控制代码之类的其他工作...我们公司这样进去的代码,感觉还挺放心的... |
74 ourcubk 2016-07-11 20:20:46 +08:00 git brunch 自动 merge into master... |
75 mahone3297 2016-07-11 21:00:57 +08:00 再来一个话题,有写 test 吗? |
76 xwartz 2016-07-11 21:28:09 +08:00 代码烂成屎,入错坑了。。 |
77 Totato5749 2016-07-11 21:30:45 +08:00 @mahone3297 哈哈哈哈 没有 |
78 sc3263 2016-07-11 22:11:54 +08:00 @kqz901002 是么?我试试。谢啦~ 其实讲道理,这东西,得项目组 leader 规定,强制执行才有效果。个人提出来,只会被认为工作量不饱和,闲着没事干。前几天在新项目组里提出来要搞一整套流程。然后被所有人反对了,理由是浪费时间。。。 |
80 hantsy 2016-07-11 22:24:16 +08:00 基本上用 Github Flow 。。。 Fork , Branch, PR, Code Review 结合 Circle CI 等,每个 Branch 都是自动运行测试,从建 PR 开始几乎团队所有人都会参与 Review ,特别与其他人有交叉的部分,@相关人,反复讨论修改。 Upstream Master 在一些项目是稳定分支,执行自动部署。没 Code Review 谁敢直接部署? |
82 metrue 2016-07-11 22:28:53 +08:00 peer to peer review. |
84 haoc 2016-07-12 06:29:41 +08:00 之前小公司,基本上自己做一 project ,人也 review 不了。在做的源目, 100+ contributors 。有 code review 和 CI 早就了吧。 |
85 webdev 2016-07-12 08:40:08 +08:00 via iPhone 只发生在找 BUG 的时候 |
87 irisLi 2016-07-12 09:27:06 +08:00 我们公司每周都要 review 代码,我觉的这样我们大代码拿出来被大家看看才会知道自己哪里需要改进,这样我们才会有进步的啊! |
88 icegreen 2016-07-12 09:36:19 +08:00 每次 merge 必须 review |
89 yanyandenuonuo 2016-07-12 09:47:29 +08:00 搭车问有哪些好的 review 的工具 :) |
90 xiaowangge 2016-07-12 10:04:25 +08:00 没有。 上周刚从 SVN 切换 到 Git 。 后端开发至少 7 人,前端开发至少 9 人。 |
91 beew 2016-07-12 10:08:53 +08:00 开发完成后自己提交一个 MR , leader 会 review ,通过了再 merge ,否则打回改到符合要求。项目稳定之后这么执行其实很好的,整套流程配合起来很顺畅 |
92 salltm 2016-07-12 10:37:08 +08:00 这么说吧。 外企基本上都有 review 的, 而且是算工作量的。 Reviewer 需要承担责任。 所以一般会比较认真 , 貌似看起来比较费时间,但是带来的产品质量提升很有效。 也少了给项目埋地雷的机会。。 国内很多公司急功近利,不重视这个, 总归是要走弯路的, 修不完的 bug 。。 |
93 Froyo9 2016-07-12 10:37:17 +08:00 都说会有,但是真正做起来的很少,哎 |
94 andyL 2016-07-12 10:38:02 +08:00 我们老大会看每次提交,一些修改会做 review ,也会对这些修改进行测试。我觉得 review 是必须的,但是 reviewer 需要是有经验有实力的 coder ,要对整个系统很熟悉。经常会给我们指出错误和提出优化意见,很棒。 |
96 Just1n 2016-07-12 12:07:53 +08:00 我们公司都是每个组组员之间互相 Review 。 |
97 ayiis 2016-07-12 12:15:58 +08:00 有,我负责 review ,我最喜欢喷人了 |
98 liuzhen 2016-07-12 12:17:19 +08:00 不定期 review |
99 cjyang1128 2016-07-12 12:23:24 +08:00 review 就跟改作业一样,老师不帮你改作业,你难道不慌嘛 |
100 hitmanx 2016-07-12 13:01:02 +08:00 有啊,在外企。经常不到10行代码 review 好几个来回耗时几天,各种挑毛病,实在没大毛病还得挑变量名啊, trailing spaces 啊之类的,痛苦得扭来扭去。 |