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

大家的工作中会有 Code Review 吗?

  •  
  •   iceAD · 311 天前 · 13461 次点击
    这是一个创建于 311 天前的主题,其中的信息可能已经有所发展或是发生改变。

    这段时间看了王争的《设计模式之美》的课程。 个人觉得里面提到的谷歌 CodeReview 很有帮助。

    • 确实有的时候需要另一双眼睛看自己的代码。

    正文中这样总结:

    让国内大部分 IT 从业人士认识到 Code Review 的重要性,形成 Code Review 的技术文化,可能还需要一个漫长的时间。不过,我特别希望,你在学完专栏之后,能够意识到 Code Review 的重要性。有朝一日,当你成了领导,有了话语权、影响力之后,能够推动在团队、公司内进行 Code Review ,甚至为 Code Review 在整个国内技术圈中发扬光大贡献一份力量。

    大家会有 Code Review 吗? 程度会到多少呢?范围会针对那些呢?

    第 1 条附言  ·  311 天前

    大概总结一下

    • 没时间 或者 公司不给时间。
    • 没必要,只有能过测试、能跑就行。

    对于中小公司来说 确实 Code Review 没太多价值,双赢的局面很难成型, 不如进一步压榨员工时间。

    115 条回复    2024-02-24 23:47:54 +08:00
    1  2  
    jedihy
        1
    jedihy  
       311 天前   ❤️ 3
    什么?难道还有公司不做 code review 的?我见过的所有公司都要求至少一个 Approval 才能 merge PR 。
    Maboroshii
        2
    Maboroshii  
       311 天前 via Android
    不做,没空。要抓紧时间让代码跑起来
    emSaVya
        3
    emSaVya  
       311 天前
    我都把 code review 放在 kpi 里作为工作的一部分了。怎么可能没有。
    tramm
        4
    tramm  
       311 天前   ❤️ 8
    @jedihy 直接批准
    zylll520
        5
    zylll520  
       311 天前
    必须有,在团队中必须推行 code review !
    yujianwjj
        6
    yujianwjj  
       311 天前
    要真正 code review 的话,至少懂别人的代码的业务。国内很多公司都是一个人干两个人的活,根本没这个精力管别人。
    leehomyhh
        7
    leehomyhh  
       311 天前
    以前有的,我领导会 review 我的代码,现在自己负责开发了,基本就是提交了扫一眼就过了
    wlfeng
        8
    wlfeng  
       311 天前
    据我所知,小公司基本是没有的
    zackzergzeng
        9
    zackzergzeng  
       311 天前
    code review 确实很有帮助,但是就是没时间,各种地方各种人都会催你尽快交付代码,能给你 review 代码的人也在忙着开发他的代码……
    darkengine
        10
    darkengine  
       311 天前
    IT 从业人士认识到 Code Review 的重要性有用吗,话事人认识到才有意义
    yigecaiji
        11
    yigecaiji  
       311 天前 via Android
    刚毕业没事会做,现在领导说我也不干,谁爱干谁干
    coderzhangsan
        12
    coderzhangsan  
       311 天前
    中小型公司基本没有,测试没问题就直接上线了,code review 只是技术角度看方案设计,有一定的积极作用,由于国内中小型公司多数业务驱动,迭代频繁的话,早期的技术设计很可能不太适应,所以国内公司产品把关的重点仍然是测试环节,只要做好测试用例,可以规避 90%以上的较大的生产事故。
    iceAD
        13
    iceAD  
    OP
       311 天前
    @coderzhangsan 这么看来,确实只适合大公司。
    billzhuang
        14
    billzhuang  
       311 天前
    我觉得 code review 比 unit test 更重要,

    如果非要二选一的话。
    SeaTac
        15
    SeaTac  
       311 天前 via iPhone   ❤️ 2
    cr review 很看人
    我遇到过极其 picky 一堆说是 nit comment 但是就是不给 approve 的
    也遇到看都不看就 approve 结果上了 pipeline 才发现问题一堆
    真正好的 cr reviewer 很少见
    kneep
        16
    kneep  
       311 天前 via iPhone
    规则都很完善且复杂,但闭着眼睛打勾的人占多数
    wu67
        17
    wu67  
       311 天前
    中小型没有.
    反正我呆过的团队, 前端仔从一组一人两人到一组 8 人, 只有一家公司有, 而且是简单走查, 没有详细 review 到很具体的逻辑, 只是大致看看没有些智障写法就给过了.
    还有一些是直接给我把 eslint 关掉的...从 origin pull 下来, 好家伙, 我本地 eslint 一堆红红黄黄的...
    28Sv0ngQfIE7Yloe
        18
    28Sv0ngQfIE7Yloe  
       311 天前
    快速迭代的业务的迭代方向往往是核心逻辑的重写,这种业务往往不会有 PR/Merge 的流程
    codingmiao
        19
    codingmiao  
       311 天前   ❤️ 1
    从刚进公司的转正报告开始,每次有和大领导汇报的机会我都要插一两页 PPT 说 CR 的事,方案、对比、收益都说过,每次领导都记下了下面就没了,哎。。之前我还在组内交叉 CR ,现在 C 个锤子,等出事了再说。
    28Sv0ngQfIE7Yloe
        20
    28Sv0ngQfIE7Yloe  
       311 天前
    @SeaTac

    「一堆说是 nit comment 」

    请问这个是什么意思 没太看懂
    BUHeF254Lpd1MH06
        21
    BUHeF254Lpd1MH06  
       311 天前
    有 假的 CR
    SeaTac
        22
    SeaTac  
       311 天前 via iPhone
    @Morii
    一般每一个 cr comment 都是需要 resolve 的(给出解释或者改掉)
    nit comment 就是可改可不改
    777777
        23
    777777  
       311 天前
    所以别用国产,都是垃圾,没人 cr 的
    Rache1
        24
    Rache1  
       311 天前
    我们有,但是,我们的主要目的是减少低级错误,不怎么关注代码质量 🙇
    mantouboji
        25
    mantouboji  
       311 天前   ❤️ 1
    Code Review 的质量度量:单位时间内听到的脏话次数。
    paopjian
        26
    paopjian  
       311 天前
    一个前端服务五个后端,改啥代码都是直接梭,爽翻
    hongweijie8
        27
    hongweijie8  
       311 天前
    小公司基本没有,review 最起码要懂业务,有的公司一个项目就一个前端一个后端,怎么 review
    Quarter
        28
    Quarter  
       311 天前 via Android
    现在项目太屎人手不够,一般人都是外包,写的很随便,没时间搞 code review ,只有 shit create
    CaptainD
        29
    CaptainD  
       311 天前
    小公司,完全没有
    liprais
        30
    liprais  
       311 天前
    水平都差不多的话能 review 出来个啥,就挑点回字有几个写法这种问题
    zxf4399
        31
    zxf4399  
       311 天前
    有的
    astray1988
        32
    astray1988  
       311 天前   ❤️ 2
    来给个 DP , 某电商跟云的北美大公司,无论是做 design 还是 code , 都有相应的 review 机制。。如果是根据业务需求来添加小的 feature/功能,需要做 design ,然后组内跟相关组都有相应的 design review ,需要组里比较 senior engineer 来 sign-off ,如果是大的业务改动(例如开一个新的 business ,对应的 org 内部有 principle engineer 的 samurai design reviews 。。所谓的 code review ,每个组内部都有相应的 code review 机制,同时 org 内部标准也会对 code review 做一定的强制要求,例如提交的 CR/PR 的 Unit Test 必须 90%以上,还要通过各种所谓的 code quality test cases...具体到各小组/或者具体的 service ,code quality 也不一样,比较 legacy 的 service 会整体对 code review 松一些, 比较新的 service 会相对高些,例如每次做个新的 feature 还要写 integration tests 来保证 full CD
    wojiaoxiaojuan
        33
    wojiaoxiaojuan  
       311 天前
    待过两家外企,都有 code review 流程。国内的小公司里,看同事 merge 就跟玩儿似的
    crazyTanuki
        34
    crazyTanuki  
       311 天前
    没有 cr ,因为没有统一培训,代码风格各异,能跑就行
    OneMan
        35
    OneMan  
       311 天前   ❤️ 1
    code review 对当好螺丝钉有用,类似茴香豆有几种写法的检查。
    对提高你的工资、职级,并没有鸟用。
    burymme11
        36
    burymme11  
       311 天前
    没时间 cr ,都是写业务,再大的公司也是 CRUD 占大头吧,中小公司就更是了,那怕每次都 cr ,时间久了,项目一样成屎山,因为需求一直迭代一直变的,业务越来越绕越繁琐,cr 的成本指数升高,而为了能过 cr ,开发写代码的成本也越来越高,死循环了。所以我司后来全面拥抱单元测试。
    我觉的核心问题要解决垃圾的产品设计和业务架构,只要这两个合理,cr 真的就啥可过的。标题中正文部分,我个人觉的不太符合大清国情。
    ren2881971
        37
    ren2881971  
       311 天前
    没卵用, 公司几十款产品,评审的人能清楚所有产品的业务逻辑么。。。 如果不清楚那 review 个毛线,只能为了提高存在感挑毛病。
    dsggnbsp
        38
    dsggnbsp  
       311 天前
    我们是草台班子
    urlpha
        39
    urlpha  
       311 天前   ❤️ 2
    油管有个很有名的视频可以搜来看看。
    “你看了,但是你对提交的具体内容并不关心。”
    flyv2x
        40
    flyv2x  
       311 天前   ❤️ 1
    国内小公司基本不重视技术文档和 Code Review 这些基本开发流程
    zogwosh
        41
    zogwosh  
       311 天前
    cr 又不算产出谁干?我连 lint 都要关就为了早点搞完早点下班
    28Sv0ngQfIE7Yloe
        42
    28Sv0ngQfIE7Yloe  
       311 天前
    @SeaTac #22

    明白了 谢谢
    smirkcat
        43
    smirkcat  
       311 天前
    自己开发,自己看,bug 自己处理,有没有都一样
    SoviaPhilo
        44
    SoviaPhilo  
       311 天前
    @burymme11 解决不了的, 产品设计和业务架构是产品部门的护城河,怎么可能容你置喙

    等业务功能一多,很多业务细节完全没人控的住的

    比起 code review , 还不如有意识地推动领域驱动设计, 减少心智成本
    Shanee
        45
    Shanee  
       311 天前   ❤️ 1
    一直有 code review ,一般提交 review 之前先要通过工具保证一些基本的代码质量例如 sonar ,checkmax ,跑完 pipeline ,sonar 的覆盖率到 80%以上,checkmarx 没有问题,才能提 PR 给人去 review ,不同的 reviewer 会 review 粒度和力度都不同,会反复 comment-》 challenge-》 resolved
    54xavier
        46
    54xavier  
       311 天前   ❤️ 3
    换了三家公司,重来没有 reviewer 过,不过自己的代码习惯还好,每次看别人的代码都是地铁老人手机脸
    nhhjhwiner
        47
    nhhjhwiner  
       311 天前
    业务需求没,技术需求有
    fqy12300
        48
    fqy12300  
       311 天前
    我一般都是自己写代码,自己测试,自己提交,自己合并,自己部署,自己发布。我还是挺相信我自己的。
    Sigrdirfa
        49
    Sigrdirfa  
       311 天前
    有点还是好的,至少能让大家知道你写了屎山,至少在 cr 变质之前可以有效防止大家写屎山。
    iovekkk
        50
    iovekkk  
       311 天前
    不仅有,还接入了 CHATGPT4 ,AI Review 给出的内容,又详细又准确
    MicroSharpAnt
        51
    MicroSharpAnt  
       311 天前
    上家公司有,而且觉得 leader 审核代码对我帮助很大。现在在一家小公司,从没 review 代码的,而且 git 大家都随便 push ,版本管理也很乱。不接手某些人的功能还好,一接手就觉得头疼,屎山就一点点堆积起来了
    NoString
        52
    NoString  
       311 天前
    我一个人手上十几个项目,哪来人 review 。基本都处于随地大小便的状态
    huyomi
        53
    huyomi  
       311 天前
    上家公司有,被大佬 review 过代码提升会很大
    twogoods
        54
    twogoods  
       311 天前
    review 确实有这种感觉,其实真没必要的东西硬是要挑点出来,直接点不会显得你水平低
    supuwoerc
        55
    supuwoerc  
       311 天前
    @iovekkk 不合规吧,我们搞过机器人,然后不然用,会泄漏代码
    pkoukk
        56
    pkoukk  
       311 天前
    会看新来的应届生的,其它基本不看
    magicls
        57
    magicls  
       311 天前
    cr 是基操吧,而且 google 还有 presubmit-verify 机制
    darkengine
        58
    darkengine  
       311 天前   ❤️ 2
    我左手一个 git merge ,右手一个 git push --force ,丝滑
    somebody1
        59
    somebody1  
       311 天前
    同一家公司干了六年了,code review 的次数没超过三次。

    国内头部 to b 网络安全企业。
    kingjpa
        60
    kingjpa  
       311 天前   ❤️ 1
    我代码都是 提交自动 webhook 到生产环境的,别说审查代码了, 我连 commit 都懒得写,每次回滚都是看时间
    TideCC
        61
    TideCC  
       311 天前
    现在一月,我们今年一年都知道要干啥活了,这个量谁给你 review
    cabing
        62
    cabing  
       311 天前
    有时候简单 cr ,但是绝大部分时候都不能 cr 出问题,因为得深入到这个业务才行。
    shizhibuyu2023
        63
    shizhibuyu2023  
       311 天前
    如果有标准,可以 lint 校验,如果没有标准,大家都是半吊子凭经验办事,没意义。
    killergun
        64
    killergun  
       311 天前   ❤️ 1
    我就是那个要 review 的人,烦人的很
    palxie
        65
    palxie  
       311 天前
    要 code review, 一般是我 review 别人的多点. 一般也是稍微看看, 有疑问讨论讨论, 但是别人 review 我的相对随便一点.
    Helsing
        66
    Helsing  
       311 天前 via iPhone   ❤️ 1
    前司有,还是很有用的,可以减少很多垃圾代码,提高代码的可维护性
    jiangzm
        67
    jiangzm  
       311 天前
    我会 CR 团队成员的,但是也提倡大家相互 review ,特别是新业务功能没有跟进很及时的话 review 不出什么问题,所以相互 review 是有意义的。 当然有公共部分/重要部分修改大家会加上我或者只指定我 reivew 。
    chiu
        68
    chiu  
       311 天前
    没有 CR approval 根本 merge 不了, 有些 repo 还需要 multiple CR approvals
    houshuu
        69
    houshuu  
       311 天前
    100% code review, 甚至 ci 改个参数都要 review.
    一方面是老人可以传递经验, 同时也能检查一些比较特殊的设计下新代码带来的功能影响.
    zeromike
        70
    zeromike  
       311 天前
    CR 是要建立氛围,形成团队对好代码、坏代码相对一致性的评判标准

    如果 CR 只有流程,没有形成有效的成果的话,基本上有和没有没什么区别。毕竟代码自测+QA 都测过了,能有什么大问题吗

    组件、工具类的还好说,对于业务代码,如果评审人在不熟悉业务的情况,很难评价,只能靠经验从代码共性上去发现问题,其实效果一般
    zypy333
        71
    zypy333  
       311 天前   ❤️ 1
    小公司,我们没有,但我会看别人的,但是只看看不说

    ps ,有没有互相 review 的网站?有时候自己写的拿不定主意,想让人帮忙提提意见,不过这家公司除了我没人在乎代码质量
    imycc
        72
    imycc  
       310 天前
    视具体人力而定。如果有专门的研发团队,且涉及多人协作的,那一定要有相应的流程规范。现在在一家大厂,围观研发团队的开发工作。几十个人共同开发一个项目,每天早上都要开 CR 会,安排了几个资深的研发来把关。
    我之前在一个四五个人的开发小组里面,每周上线前也会做内部 CR 。但后来去一家中小公司,就我跟另一个半桶水的同事,我会给他 CR ,但他给我 CR 就不指望了。有时候也挺无助的。
    kangyue9999
        73
    kangyue9999  
       310 天前 via Android
    没有 CR 的公司是不是也没有测试?所以最后就是一坨答辩?开发到一定程度就变成不停在造 bug 和修 bug 。然后一单有人离职这代码他的部分多半有可能别人就没法维护了。。。
    Hyvi
        74
    Hyvi  
       310 天前 via iPhone   ❤️ 1
    很重要,but 投入和价值远比不上业务带来价值呀
    neroxps
        75
    neroxps  
       310 天前 via iPhone
    哈哈哈 我司可能测试都不全。写完就上线。boom 了就现场顶住客户的怒火
    kinghly
        76
    kinghly  
       310 天前 via Android
    我基本都会 review 的,有问题必须改完才会通过。
    yzbythesea
        77
    yzbythesea  
       310 天前   ❤️ 1
    CR 是基操吧。。。“没时间” 写 CR ,感觉就像没时间擦屁股一样滑稽。
    JounQin
        78
    JounQin  
       310 天前 via iPhone
    曾经给团队的小朋友的 MR 提过一个最后回复超过 100+ 的 review ,不知道他恨不恨我。

    放一个可以公开的正在进行中的 PR review ,目前已经有 50+ conversation 了。
    JounQin
        79
    JounQin  
       310 天前 via iPhone   ❤️ 1
    @Hyvi 磨刀不误砍柴工,很多人/公司不懂这个道理,好的代码扩展性很强、维护成本低很多,不能光顾着眼前的一丁点儿时间。
    JounQin
        80
    JounQin  
       310 天前 via iPhone   ❤️ 1
    https://github.com/alauda/ui/pull/533

    刚发现 PR 链接漏发了。🥲
    luzemin
        81
    luzemin  
       310 天前   ❤️ 1
    有,至少 2 个 approval 才能 merge 。

    不过因为各种原因,仍然还是有一些问题:

    1. 小团体,我提了代码我想快速 merge ,那我就私下找关系好的开发,“帮我 review 下”言下之意“帮我点一下 approval ”

    2. 随便一个 story/task ,都会修改很多文件,review 只能粗看

    3. 加了新的 solution ,不是所有人对原有的业务一清二楚,所以基本上 review 一些规范上的问题,很少 review 实现上的逻辑组织和代码复用等问题。
    thomas15425
        82
    thomas15425  
       310 天前
    公司没有 code review ,就连 Test 都没有,写完直接 push 就行。现在那代码是十几年的屎山,bug 还一大堆。公司完全没有要重构的意思,还有往里加新功能。

    之前的人不用的代码直接注释掉就完事。
    有些代码只写了函数的外壳,里面没有代码。
    写一大堆没有用的变量
    非必要的指针有大堆,内存溢出也不管
    index 有些由 0 开始,有些由 1 开始。
    代码的金字塔能建到 7-8 层
    UIXX
        83
    UIXX  
       310 天前
    以前做现在不做,一是投入产出比的问题,二是无法做到有效 CR 。

    具体情况是,研发有各人的专业窄道,不是同样方向同等水平的研究者很难看懂对方的代码,也谈不上什么 CR 了。
    assad
        84
    assad  
       310 天前   ❤️ 1
    别人业务都做了好几轮了,你这个还没 review 完。还得看情况而定
    zeromake
        85
    zeromake  
       310 天前
    和上面的兄弟说的一样,没法做有效 cr ,业务一复杂,一个 pr 就是 30-50+ 文件变更,然后再加上你完全不知道同事在写的业务是什么(最多知道他写的业务叫什么),能看出什么东西呢?看什么语法注释各种规范标准?这种东西能总结出来应该完全可以做成 lint ci 工具了,根本轮不到人看……
    rocksolid
        86
    rocksolid  
       310 天前
    不理解 review 的意义,除非通用功能,否则你能指望另一个同事跟了几周几月的逻辑你几个小时弄懂么,代码质量规范可以用 sonar 之类的
    Promtheus
        87
    Promtheus  
       310 天前
    做是要做的,但是有点形式主义。感觉做了和没做也没啥区别。也就找找写法错误优化优化 很少回去看架构设计。review 的人也是抱着完成任务的态度。凑几个点提交完事。逼近这个花太多时间也不会给自己加钱,谁愿意认真看别人的代码呢
    fredweili
        88
    fredweili  
       310 天前
    有,这东西不是平等的,就是有经验的帮没经验的挑错
    unco020511
        89
    unco020511  
       310 天前
    有,我们是合代码之前必须有两个人点,不过基本都不会看得很仔细
    Torpedo
        90
    Torpedo  
       310 天前
    cr 这种和你的项目、组织架构息息相关。
    cr 相当于让另一个人理解一遍你的代码。
    付出最多的是另一个不做你需求的人,无形中要求这个需求的人数增加
    但是实际很多项目都是只有一个人负责,没有关系他做了什么。只要他交付需求就可以了
    这样的组织架构显然不可能有什么有效的 cr

    相反,比较基础、核心的组件开发设计都有多人共同维护,这样 cr 才有基础
    bzw875
        91
    bzw875  
       310 天前
    没见过不做 code review 的公司,除非小作坊、小公司。我们还做 ESlint ,还要写接口要写测试用例
    zw1one
        92
    zw1one  
       310 天前
    作为负责人的角色,我会要求组员开发前讲解下代码设计,设计上没问题就行了。
    新来的小朋友我会 review 一下他的代码,或者叫同事盯一下,免得挖坑。
    合作比较愉快的同事,没必要 review ,太形式了,代码设计聊好就行了。
    MEIerer
        93
    MEIerer  
       310 天前
    有没有 code review 得看公司有没有规范吧,正好国内的公司很多都是草台班子,不 review 也是正常的
    hevi
        94
    hevi  
       310 天前
    我所在小公司,连需求都没 review:D
    bajitanglang
        95
    bajitanglang  
       310 天前
    一句话,老板要某某时间就上线.
    JounQin
        96
    JounQin  
       310 天前
    @rocksolid review 从来不是看具体业务代码,而是看你的逻辑代码,好的代码是一脉相承的,跟业务没关系。
    JounQin
        97
    JounQin  
       310 天前
    @rocksolid 我 review 代码从来不看具体业务是什么,这跟我没关系,我只管写出来的代码是不是解耦优雅,至于你说的 sonar lint 那些只能解决很少的代码异味,更重要的是编码思维。
    JounQin
        98
    JounQin  
       310 天前
    @Promtheus 只能说给你 review 的人很不用心。
    JackSlowFcck
        99
    JackSlowFcck  
       310 天前
    @darkengine
    说到点上了,得生意人认识到才有用,毕竟谁会在乎资源的想法?
    superchijinpeng
        100
    superchijinpeng  
       310 天前
    每一个 Merge Request 都要 Code Review
    1  2  
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   1709 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 38ms · UTC 16:45 · PVG 00:45 · LAX 08:45 · JFK 11:45
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.