V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
ha2vex
V2EX  ›  程序员

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

  •  
  •   ha2vex · 2020-04-13 12:07:10 +08:00 · 4932 次点击
    这是一个创建于 1685 天前的主题,其中的信息可能已经有所发展或是发生改变。
    第 1 条附言  ·  2020-04-13 14:52:25 +08:00
    主要是 Review 下代码是否按规范编写,有没有低效可优化代码这两种。
    组织形式上,几个人听一个人讲自己写的部分代码,这种挺费时间,效率不高。
    人手多了,写的代码各种“骚操作”。
    27 条回复    2020-04-17 09:57:32 +08:00
    ZSeptember
        1
    ZSeptember  
       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
    我们就是开个小会,大家过一下代码。其实代码看起来多,并没有多少,而且发现的大部分问题都是一些复用的问题,代码格式和习惯统一的问题。
    SpencerCJH
        7
    SpencerCJH  
       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 永远是上级做的事,自己代码提交上去就不管了。
    ladypxy
        16
    ladypxy  
       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   ❤️ 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   ·   实用小工具   ·   2818 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 71ms · UTC 07:38 · PVG 15:38 · LAX 23:38 · JFK 02:38
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.