大家在公司都是怎么组织 Code Review 的?高效吗?有效果吗? - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
请不要在回答技术问题时复制粘贴 AI 生成的内容
ha2vex
V2EX    程序员

大家在公司都是怎么组织 Code Review 的?高效吗?有效果吗?

  •  
  •   ha2vex 2020-04-13 12:07:10 +08:00 5424 次点击
    是一个创建于 2008 天前的主题,其中的信息可能已经有所发展或是发生改变。
    第 1 条附言    2020-04-13 14:52:25 +08:00
    主要是 Review 下代码是否按规范编写,有没有低效可优化代码这两种。
    组织形式上,几个人听一个人讲自己写的部分代码,这种挺费时间,效率不高。
    人手多了,写的代码各种“骚操作”。
    27 条回复    2020-04-17 09:57:32 +08:00
    looplj
        1
    looplj  
       2020-04-13 13:57:49 +08:00
    组织?
    一般就是合并代码的时候 Review
    如果大家都认真的话,确实很耗时间,有时候会 block,不过整体对团队,对自己,对项目都是有好处的
    我们现在的做法是,先提交模块 interface 定义,大家 review,定好接口后,简单方法的具体实现不会 reivew 的很细
    可以定个 reivew checklist,定好 reivew 需要关注哪些方面,不然 review 的时候很容易发散,就浪费时间了
    lovedebug
        2
    lovedebug  
       2020-04-13 13:59:52 +08:00
    团队内的小 group 可以组织 review 。
    但我们一般是在 github flow 上执行的,向 INT 分支提交代码必须 2 人以上 review 。
    另外同 1 楼一样,有一个 common 的 review checklist,主要涉及到现有架构、业务模型和工作流上的需要注意的 review 点
    Mithril
        3
    Mithril  
       2020-04-13 14:10:58 +08:00   2
    最近发现 Code Review 实际上只能针对 Coding Rule 或者架构设计方面进行 Review,真正的实现细节,业务逻辑基本上是很难 Review 出问题的。某些逻辑错误不是真正需求实现者很难在短暂的 Review 中考虑到。最好还是 TDD,商量好接口以后指派两人分别开发 UT 和实现逻辑,Review 只是辅助。但这在很多开发资源紧张的情况下难以实现。
    hantsy
        4
    hantsy  
       2020-04-13 14:20:26 +08:00
    github flow
    hantsy
        5
    hantsy  
       2020-04-13 14:21:52 +08:00
    @Mithril 业务逻辑的正确依赖写测试,CI ( PR 分支跑 CI 自动化)。
    CBS
        6
    CBS  
       2020-04-13 14:23:56 +08:00
    我们就是开个小会,大家过一下代码。其实代码看起来多,并没有多少,而且发现的大部分问题都是一些复用的问题,代码格式和习惯统一的问题。
        7
    0dJ6Tu8Za734L89T  
       2020-04-13 15:12:13 +08:00
    我这边所有的开发都要在支线分支进行,完成了要提交 merge request,需要有高一级别的工程师(一般是组长,也可能是别的组的)来 review 代码.

    如果没有时间一行行看,也要确保你心里想的和他心里想的,和需求的要求是一致的.

    code review 是一个对人要求很高的工作,我以前公司也组织过,那时候是一个周期来一次 review,结果因为同事的水平实在太低,每次 review 都像是在教怎么写 Java,就没什么意思.
    zclHIT
        8
    zclHIT  
       2020-04-13 15:20:41 +08:00
    我认为 code review 更多的是 knowledge transfer 的作用,无论是业务 context 的传递,以及“优秀实践”的分享
    VDimos
        9
    VDimos  
       2020-04-13 15:21:33 +08:00 via Android
    gerrit
    server
        10
    server  
       2020-04-13 15:25:35 +08:00
    @zclHIT 确实,看着是一组人,个个单兵作战
    Mithril
        11
    Mithril  
       2020-04-13 15:43:22 +08:00
    @hantsy 问题在于这个测试不能由同一个人写,不然基本都会变成对自己写好的业务逻辑进行测试。但是某些情况考虑不到的话就还是会漏掉。
    所以要么换人写,要么让 PM 去 Review 这些 UT 。但是前提是 PM 能看懂代码,而且有基本的逻辑思维能力。
    实际上很多 PM 的逻辑思维能力感人,很难考虑到自己的需求都有哪些极端情况。
    twoconk
        12
    twoconk  
       2020-04-13 15:50:34 +08:00
    记得多年前在 HW 的代码 Review,更多时候是主讲的人,在讲代码包括逻辑、包括架构的实现过程中,讲的人发现代码的问题,别人发现的不多;提高效率的方法,主要是通过提前将修改前后的代码发出来,检视人提前了解代码的实现逻辑;
    Arisky
        13
    Arisky  
       2020-04-13 15:57:17 +08:00
    upsource 很好用!
    但是实践上确实是个难题。感觉高质量的 review 基本也快把内容重新实现一遍了。。
    hantsy
        14
    hantsy  
       2020-04-13 19:55:59 +08:00
    @Mithril 单元测试,集成测试必须是程序员自己写的。今天我还第一次听说测试要由别人写。

    一些大型公司有专门测试人员,写的测试只能是 UAT 阶段写的 Functional Test 代替手动测试功能,主要从需求角度进行黑盒测试,验证所有路径是否符合预期。
    hantsy
        15
    hantsy  
       2020-04-13 20:00:33 +08:00
    @zclHIT 没错,Code review 更多的时候叫做 Peer Code Review,也可以当成 Pair Programming 的一种表现形式。相互 Code Review,主要是交流经验。国内程序员都是习惯了家长性的检查,认为 Code Review 永远是上级做的事,自己代码提交上去就不管了。
    Dragonish3600
        16
    Dragonish3600  
       2020-04-13 20:02:44 +08:00 via iPhone
    任何一个 change 需要至少 2 个 review,approve 后才能合并
    hantsy
        17
    hantsy  
       2020-04-13 20:06:04 +08:00
    @server 以前我在公司上班的时候,经常听到的一个笑话,某个人呆了几年资历比较老的程序员在项目里,经常说一看什么代码风格就知道谁写的。我不得不说对于一个团队这是悲哀的。一个团队只应该有一种 Code Style,所有的 Bad Smell 应该在 Code Review 提出来干掉。有的人讲重构,讲代码质量说得头头是道,自己做的时候完全忘到一边了。
    hantsy
        18
    hantsy  
       2020-04-13 20:09:00 +08:00   1
    @Arisky Jetbrains 的一套工具的确强大。TeamCity,Upsource,YourTrack, Space.

    对于小团队很合适, 不是说不大团队不行,主要它的 License 限制,小团队可以免费。
    fewok
        19
    fewok  
       2020-04-13 20:56:57 +08:00
    所以,出个事故,开发这块代码的人请假或离职。你们打算重新读代码,然后一行一行的排查问题嘛?
    OakScript
        20
    OakScript  
       2020-04-13 21:12:10 +08:00
    大多数公司业务驱动,换句话说业务都写不完,CR 大多数都是走个形式,能大概过一眼,点个 approval 就不错了
    jianghu52
        21
    jianghu52  
       2020-04-13 22:35:59 +08:00 &nsp; 1
    我总结了 code review 的两个凡是
    凡是能坚持半年以上的,项目通常都进行的不错.
    凡是能总结出文档的,通常项目都有牛人.
    反之则是
    凡是坚持不到半年的,项目通常都已经开始有臭味了
    凡是 code review 之后啥都没留下来的,最后就都不了了之了.

    code review 这个东西,有点像内功一样,平时你看不出来效果,还贼费时费力,当你真的遇到一个大坑的时候,才会觉得,code review 是多么的必要.但是呢,真到那个时候了,能坚持 code review 的人很多时候要么是愤而离开,要么就是开始随波逐流了.
    Blaky
        22
    Blaky  
       2020-04-13 22:53:01 +08:00
    @jianghu52 "能坚持 code review 的人很多时候要么是愤而离开,要么就是开始随波逐流了", 老哥这句话扎心了,我就是那个愤而离开的
    ha2vex
        23
    ha2vex  
    OP
       2020-04-14 09:27:28 +08:00
    @jianghu52 文档输出很重要,作为标准依据。
    人多了执行下来就困难,也就是为啥要 Review 了,还有新人还要培训等等。
    ha2vex
        24
    ha2vex  
    OP
       2020-04-14 09:31:20 +08:00
    @zclHIT 很同意,“优秀实践”,就是很多经验上的总结。
    wzzzx
        25
    wzzzx  
       2020-04-14 13:02:42 +08:00
    @jianghu52 总结出文档?这个怎么理解?
    maisui99
        26
    maisui99  
       2020-04-14 15:46:16 +08:00
    发布加个 code review 卡口。。
    然后定期挑选随机挑选变更进行 code review 会晒一下
    jianghu52
        27
    jianghu52  
       2020-04-17 09:57:32 +08:00
    @wzzzx code review 是一项精进的工作,那么如何保证精进,而不是在原地踏步呢,只有靠文档.当我们 code review 过程中,发现以前遇到同样的错误的时候,这个时候,如果有整理的文档,那么马上就可以说,看,这个错误是之前我们总结的错误的第几条. 只有这样慢慢积累,code review 才是一个质量提升的过程,而不是一个浪费大家时间,只有投入,没产出的环节.
    关于     帮助文档     自助推广系统     博客     API     FAQ     Solana     2401 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 26ms UTC 15:39 PVG 23:39 LAX 08:39 JFK 11:39
    Do have faith in what you're doing.
    ubao snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86