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

现实是 Code Review 只会拖慢项目进度,真正的隐患往往是 Review 之后合并进了主分支,而很久之后才被发现

  •  
  •   zqx · 2019-08-24 17:42:42 +08:00 via Android · 5948 次点击
    这是一个创建于 1678 天前的主题,其中的信息可能已经有所发展或是发生改变。
    周五上午我把自己的所有 Bug 修好,提 PR,喊同事 Review (Bitbucket 设置必须有 2 人批准,才能合进发布分支)
    然而今天突然被叫起来修 Bug,我想着昨天都搞完了呀。一看是昨天的 PR 还没合并,赶紧再喊同事 Review,这种形式主义的 Code Review 有毛线作用?
    29 条回复    2019-08-26 13:56:22 +08:00
    zqx
        1
    zqx  
    OP
       2019-08-24 17:49:40 +08:00 via Android
    我在 PR 阶段被提出的最多问题就是: 变量命名之类的问题。我认为这类属于协作效率问题,不应该在某个具体 bugfix 的 PR 阶段提出,而应该是每周或每月开小组技术会议的时候一起 check。真正应该关注的是: diff 中是否改动了无关代码,改动部分的影响范围,当然还有程序逻辑是否正确,会不会产生更多潜在的 Bug。
    zqx
        2
    zqx  
    OP
       2019-08-24 17:56:29 +08:00 via Android
    Code Review 的好处太多了,但那只有硅谷或者国内小部分工程师文化的公司才能享受,大多数不那么工程师文化的大厂最后演变成了: 上级要求下级 Approve PR,下级一般会假装看代码,然后 Approve ;同级之间 Code Review,就看互相关系好不好,你讨厌一个程序员,那就在他的 PR 中挑毛病吧。
    JamesR
        3
    JamesR  
       2019-08-24 17:56:30 +08:00
    只能想办法加快审了。
    我绝大多数 Bug 都是程序跑上 3 个月以上才能暴露,哈哈。
    Jiavwen
        4
    Jiavwen  
       2019-08-24 18:40:34 +08:00
    你每次 PR 是否有足够的测试覆盖?如果没有,那么合并之后出现 bug 也是正常的
    1424659514
        5
    1424659514  
       2019-08-24 18:51:01 +08:00
    @JamesR 🐂🍺
    zqx
        6
    zqx  
    OP
       2019-08-24 19:27:10 +08:00 via Android
    @Jiavwen 前端,涉及逻辑的部分是有单元测试的,其他 UI 相关的都是人工检查,这部分很难用程序逻辑去描述是否正确
    arrow8899
        7
    arrow8899  
       2019-08-24 19:29:02 +08:00
    只能说明你们 Code Review 没做对,不能说 Code Review 没用; Code Review 是用来改进代码质量和知识分享的,至于代码风格、BUG 等,应该由对应的代码质量检测工具和单元测试来做。
    hoyixi
        8
    hoyixi  
       2019-08-24 19:48:32 +08:00   ❤️ 2
    Code Review 没问题,而是你们自己特色的 Code Review 有问题
    fonlan
        9
    fonlan  
       2019-08-24 21:55:48 +08:00 via Android
    流程不规范
    kaedea
        10
    kaedea  
       2019-08-24 21:59:28 +08:00 via Android   ❤️ 2
    平均研发素质不达标的队伍不适合 Code Review,特别是 bat 里的队伍
    jedihy
        11
    jedihy  
       2019-08-25 03:44:30 +08:00 via iPhone
    代码有 bug 不能怪 CR 的。而且,项目进度问题是你们项目规划本身就做得不好。
    zqx
        12
    zqx  
    OP
       2019-08-25 04:57:33 +08:00 via Android
    各位,我说了 Code Review 是好的,但是多数公司包括大厂也一样,最终会把技术问题演变成流程问题。
    关于流程,Git Flow 工作流,双周迭代(固定隔周的星期二发布窗口),很多项目管理的细节都贴近敏捷软件开发。都是最佳实践啊,哪里出问题了?
    我觉得还是人的问题,平均素质差(无论技术上还是价值观上)的即使用了最佳实践,结果也不一定好
    wd
        13
    wd  
       2019-08-25 06:58:39 +08:00 via iPhone
    一把刀拿来杀猪还是削苹果那当然是你们自己的事情,关刀屁事
    justfortest
        14
    justfortest  
       2019-08-25 08:38:25 +08:00
    Code Review 真的挺难实行的,毕竟不是每个人都对逻辑很清楚,而且很多团队所谓的 Code Review 只是拉几个人开会而已,并不能发现什么问题,作用不大,相比 Code Review 我认为单元测试、集成测试的作用更大。
    zqx
        15
    zqx  
    OP
       2019-08-25 08:44:56 +08:00 via Android
    @wd 在养猪场削苹果和苹果园杀猪,都不合适,大多数公司就是这种环境。
    Antihank
        16
    Antihank  
       2019-08-25 09:35:13 +08:00   ❤️ 1
    不 review 的你难以想象你的同事能写出多么愚蠢的代码,做出多么臃肿的设计。
    razertory
        17
    razertory  
       2019-08-25 10:41:41 +08:00
    Code Review 有时候需要一定程度配合 Unit Test
    seki
        18
    seki  
       2019-08-25 10:58:10 +08:00
    ui 相关的当然可以自动测啊,e2e,image diff 之类的,有总好过没有
    xiubin
        19
    xiubin  
       2019-08-25 11:03:13 +08:00
    当需求都写不完的时候,不计入工时的 CR 都是耍流氓

    最多也就是看看命名或者单独的方法内部逻辑
    GoLand
        20
    GoLand  
       2019-08-25 11:18:15 +08:00
    reviewer 需要有责任心,先理解业务需求是什么,并且尝试理解同事的逻辑,尽量找出逻辑 bug 以及一些代码层面的 bug。反正我 review 同事 PR 的时候,都把需求当成自己的需求,首先我会想一想同事的实现方式有没有问题,如果是我我会怎么实现,是不是比这个实现方式更好,如果更好那么我会提出来;然后我会详细看 diff,不要吝啬发 comment。

    大多数人如果只有自己一个人写代码,没有 review 是很容易出错的,写个 typo 啥的,逻辑进到一个死胡同,是经常的事吧,很多时候自己想半天都没觉得有错,而别人一看就知道有错。
    celeron533
        21
    celeron533  
       2019-08-25 12:55:49 +08:00
    Code review 内容有好几种:
    1. 看拼写、规范
    比如 typo (还不是那种自动拼写检查能查出来的那种),使用了错误的变量,缩进不合格,注释不详细等
    2. 看业务
    这个真的只有看需求和经验了。明明这个操作要把购物车里所有的东西打折,你却把收藏夹打折
    3. 看代码实现
    “再开个线程,不要阻塞 UI ”
    4. 排雷
    “下个月我离职,所以这段代码在 3 个月后会删库”
    ParadiseDS
        22
    ParadiseDS  
       2019-08-25 13:36:24 +08:00 via Android
    review 配合单测很重要,review 实现的时候,看的基本是大概的框架和流程,功能逻辑的正确性交给单测保障,流程没大问题直接去看单测方案是否全面,所以我个人在单测的要求和 review 单测的精力往往投入更多
    fuyufjh
        23
    fuyufjh  
       2019-08-25 14:18:59 +08:00
    为啥要搞成 2 人批准?个人觉得 1 人足够了
    middleware
        24
    middleware  
       2019-08-25 15:06:01 +08:00
    Code review 是为了保证你的 code 不会违反一些原则(比如 single source of truth, don't repeat yourself )。这样以后发现问题更好处理。而不是保证没有 bug。
    Justin13
        25
    Justin13  
       2019-08-25 18:02:04 +08:00 via Android
    真正的隐患往往是 Review 之后合并进了主分支,而很久之后才被发现

    幸存者偏差
    weixiangzhe
        26
    weixiangzhe  
       2019-08-25 18:03:11 +08:00
    git lab code review 的时候只能看到修改与新增的东西,没有看到完整代码和业务逻辑也很难明白对方是在做什么,而且大家都这么忙,感觉只能看看风格和明显的逻辑错误这种东西了吧;但是 review 一下确实也能学习一下对象的思路啥的
    ZiLong
        27
    ZiLong  
       2019-08-26 12:03:40 +08:00
    @weixiangzhe 羡慕你们能学习对象的思路的
    weixiangzhe
        28
    weixiangzhe  
       2019-08-26 12:39:45 +08:00 via iPhone
    @ZiLong 打错字啦 不搞🐔的
    ZiLong
        29
    ZiLong  
       2019-08-26 13:56:22 +08:00
    @weixiangzhe 不搞🐔的,我们只是 do chicken right!
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   我们的愿景   ·   实用小工具   ·   5329 人在线   最高记录 6543   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 28ms · UTC 08:25 · PVG 16:25 · LAX 01:25 · JFK 04:25
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.