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

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

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

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

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

    幸存者偏差
        26
    weixiangzhe   57 天前
    git lab code review 的时候只能看到修改与新增的东西,没有看到完整代码和业务逻辑也很难明白对方是在做什么,而且大家都这么忙,感觉只能看看风格和明显的逻辑错误这种东西了吧;但是 review 一下确实也能学习一下对象的思路啥的
        27
    ZiLong   57 天前
    @weixiangzhe 羡慕你们能学习对象的思路的
        28
    weixiangzhe   57 天前 via iPhone
    @ZiLong 打错字啦 不搞🐔的
        29
    ZiLong   56 天前
    @weixiangzhe 不搞🐔的,我们只是 do chicken right!
    关于   ·   FAQ   ·   API   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   3304 人在线   最高记录 5043   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.3 · 26ms · UTC 04:43 · PVG 12:43 · LAX 21:43 · JFK 00:43
    ♥ Do have faith in what you're doing.