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

大部分公司应该都不会 code review 吧?

  •  
  •   polyang · 2021-01-04 09:55:15 +08:00 · 11017 次点击
    这是一个创建于 1426 天前的主题,其中的信息可能已经有所发展或是发生改变。
    以我所在的公司为例,平时项目周期已经很紧张了,根本没时间 code review 。

    之前有个需求,从开发到联调再到 showcase,只给你一周的时间,这么短的时间,根本不会考虑什么 code review,只要能把功能实现就不错了,等到做完这个需求,还没等你缓过来,下一个需求就立马来了。
    71 条回复    2021-01-05 18:13:33 +08:00
    x66
        1
    x66  
       2021-01-04 10:03:59 +08:00   ❤️ 2
    我司有这个流程,每次提交代码都会有两位同事和一位 team leader review 。
    但是很多时候也会沦为一种形式,因为同事写的业务你并不一定都清楚,也只能帮忙看看语法有没有明显的问题。
    有些同事一次改动的代码过多,很难 review,
    更多的时候你自己思路来了正在 coding,同事来找你 review,很难放下自己手里的工作去仔细看他的代码
    ferock
        2
    ferock  
       2021-01-04 10:14:24 +08:00   ❤️ 1
    楼上+1

    参考鹅厂,有 code review,svn 递交必须审批,回顾问题落实到责任制。
    那可能沦为形式化会弱一点。


    制度和人之间,关键作用的还是人。
    Mirage09
        3
    Mirage09  
       2021-01-04 10:17:36 +08:00 via iPhone
    看情况吧,小的 CR 走个形式,大的 CR 还是比较严肃的
    NexTooo
        4
    NexTooo  
       2021-01-04 10:17:37 +08:00   ❤️ 6
    因为安排了 code review 的任务,也不会给你 review 的时间……
    duduaba
        5
    duduaba  
       2021-01-04 10:19:18 +08:00
    code review 最大的阻塞就是很大的 commit,如果各个功能改动细分 commit,那 review 就很容易了。
    jzmws
        6
    jzmws  
       2021-01-04 10:26:45 +08:00
    小公司根本没办法做到 CR ,有时候看看别人的代码最多看看代码是否符合规范,注释啥子是否描述正确,其他的真的不好再弄
    jdhao
        7
    jdhao  
       2021-01-04 10:29:07 +08:00
    能跑就行。
    gggxxxx
        8
    gggxxxx  
       2021-01-04 10:30:24 +08:00   ❤️ 2
    code review 只在 beta 时期有用。软件大部分都完工了,这个时候任何的代码修改,大家一起开会 review 看看,看看会不会因为个人修改代码考虑太少造成其他部分额外的风险。
    我一直很不认同互相 review 代码风格啊,具体实现啊。毫无意义的事情。一方面,别人看你代码可能根本看不懂,每个人负责的模块和业务都不一样,草草看看也不实际测试,然后提交一个已阅标记........给自己一种感觉仿佛好像自己是美帝互联网大厂一员.....
    另一方面代码风格其实根本不重要,只要能正确运行的代码就是好代码。我曾见过真正美帝互联网巨头公司的员工代码,每个人都是风格各异性格突出,有些人就喜欢 tab 空 2 个 space 呢.....
    xuboying
        9
    xuboying  
       2021-01-04 10:37:39 +08:00
    @gggxxxx #8 代码风格不是项目一开始就定好了么,机器检查,项目风格都定不下来不是技术负责人压不住小弟么。要么就是没有负责人。。。
    chiu
        10
    chiu  
       2021-01-04 10:37:44 +08:00   ❤️ 1
    merge 代码的流程就有 review (自己 review/verify -> 机器人 review+机器人 build+机器人 test -> leader/owner review), 以上整个流程通过后, change 才会自动 merge 到对应 branch 上
    saulshao
        11
    saulshao  
       2021-01-04 10:40:33 +08:00   ❤️ 3
    代码 review 的核心目标其实是尝试让其他人理解作者的代码。
    挑刺永远都比做事容易,如果各位理解这点,就会明白 code review 其实是非常重要的,应该要切实执行。
    gggxxxx
        12
    gggxxxx  
       2021-01-04 10:51:01 +08:00
    @xuboying 为啥老是开口就是权力话语这套说辞呢....压不住小弟....喷了。
    一个团队里的成员有自己的个性其实是好事,在不重要的事情上做各种约束其实意义不大。想想看,团队里每个人都穿格子衬衫开发效率就会提高么?
    xuboying
        13
    xuboying  
       2021-01-04 10:54:31 +08:00
    @gggxxxx #12 只是一份工作而已,不值得展示个性。没人在意
    Lemeng
        14
    Lemeng  
       2021-01-04 10:55:18 +08:00
    没有绝对的,重视程度有大不同
    ericcode
        15
    ericcode  
       2021-01-04 10:55:25 +08:00
    来这家公司 3 年了,一直坚持做 review,很有用,但是真的很费时间
    b1ackjack
        16
    b1ackjack  
       2021-01-04 10:56:43 +08:00
    之前小公司 sonar 和人工 cr 都有,来了这二线啥都没了
    hantsy
        17
    hantsy  
       2021-01-04 10:58:45 +08:00
    @chiu 这个流程差不多可以。

    我现在只要参与的项目 100%需要 CR 。主要创建的 PR 在进行。

    进行 Code Review 之前,必须保证(这些都是由 CI 完成):
    1,所有代码通过测试
    2,测试 Coverage 报告(指标认可, 测试不到位,添加测试)
    3,代码质量检测工具报告达标(如果引入新的 Bad Smell 代码等,需要重构清除)(现在智能化的检测工具,借助大数据,通过海量分析,可以检测很多代码质量的问题,及预测运行时产生的动态问题,不仅仅是命名规范等的问题)

    这之后才是人工 Code Review,加入一些修正意见
    4,Code Review,进行重构

    还是重申我的观点,不写测试,做 CR,CI 都会成为形式主义。
    tkl
        18
    tkl  
       2021-01-04 11:07:40 +08:00
    1L + 10L

    sonarqube
    channingcheng
        19
    channingcheng  
       2021-01-04 11:19:59 +08:00
    @saulshao 说的太对了, 遇到一个总挑别人的人,什么都要按照他的想法来实现,是时候真不想搭理这人
    channingcheng
        20
    channingcheng  
       2021-01-04 11:21:36 +08:00
    @hantsy 有个问题,cr 后重构,那测试不就要重新测了?确认改那么多东西没有问题吗?
    timedivision
        21
    timedivision  
       2021-01-04 11:25:41 +08:00   ❤️ 1
    @gggxxxx 代码风格不定好,拉代码合并的时候,你 2 个 space,他一个 tab,你咋合并?难道自己不用格式化代码的吗
    Orenoid
        22
    Orenoid  
       2021-01-04 11:33:54 +08:00
    我司是有 CR 的,小型创业公司。
    虽然谈不上形式化,但这个流程目前基本上已经变成,以 “应该按我的想法来实现” 为目的了。反正我现在也懒得争论了,对方怎么说我就怎么改……说到底还是话语权的问题,总该有个人让步。
    taowen
        23
    taowen  
       2021-01-04 11:45:31 +08:00
    code review 只是一种给反馈的手段而已。如果能尽早提出改进意见,没必要等到 code review 的时候来做。cr 时候提一堆意见去返工,浪费也挺大的。
    weer0026
        24
    weer0026  
       2021-01-04 11:47:41 +08:00
    小公司表示只有在出现性能问题才会去 code review,新员工刚来也会看看,感觉大部分小公司都这样。
    IsaacYoung
        25
    IsaacYoung  
       2021-01-04 11:47:47 +08:00
    走形式
    mazyi
        26
    mazyi  
       2021-01-04 11:59:33 +08:00 via iPhone
    垃圾人做辣鸡事,都是垃圾业务怎么来厉害的技术呢?
    Tonni
        27
    Tonni  
       2021-01-04 12:10:39 +08:00
    我们公司会,Code Review + CI + Unit Testing,就是产品做的一塌糊涂
    saulshao
        28
    saulshao  
       2021-01-04 12:20:21 +08:00
    @channingcheng
    19# 我想表达的愿意其实是写代码的都不喜欢被人 Review,而参与 code review 的人都应该会喜欢。
    所以逻辑上应该所有开发都参与 code review,这其实不是看有多少正面效果,而是增强互相之间的理解和沟通。
    hantsy
        29
    hantsy  
       2021-01-04 12:26:43 +08:00
    @channingcheng 你从来没用过 CI 吗???
    hantsy
        30
    hantsy  
       2021-01-04 12:28:16 +08:00   ❤️ 1
    @saulshao 这一点没错。更多应该 Peer to Peer Code Review,建立有效的沟通机制,而不是什么上级 Review 下面的人的代码。
    Cstone
        31
    Cstone  
       2021-01-04 12:42:56 +08:00
    以前有,几个开发大家约个会议室一起认真评审代码,后来来了个领导,说减少工作时间开大会私下指定人 review 就行,代码评审也就成了走过场,再到现在没人 review 了
    leega0
        32
    leega0  
       2021-01-04 12:50:56 +08:00
    小公司就如你所说,开发周期根本没有 review 的时间给你,基本匆匆上线,各种改 bug 再测,循环往复。
    dayeye2006199
        33
    dayeye2006199  
       2021-01-04 12:51:16 +08:00   ❤️ 4
    我们公司只有三个人,也有 code review,有 CICD,有测试覆盖需求。这是一个企业文化的问题,对这点我们不妥协。
    lights
        34
    lights  
       2021-01-04 13:00:25 +08:00
    刚毕业在传统 IT 行业的时候,没有 CR
    后来跳槽做了两年多的互联网,配合 gitlab 做 CR 感觉很棒,偶尔能学到新姿势,也有利于增强代码维护性
    再后来转行做游戏,虽然都是用 SVN,但第一家公司有做简陋的 CR,就是看一遍再用 QQ 或者口头和你沟通
    现在依旧做游戏,没有 CR,虽然公司小,但毕竟有不同的人做同一个模块,偶尔会觉得比较蛋疼
    hantsy
        35
    hantsy  
       2021-01-04 13:16:14 +08:00
    @dayeye2006199 这点没错,不用 CI,不写测试,所有 CR 都会是形式。

    那种开会形式讨论,什么盯着纯代码提意见的,我也经历过,没任何实质性的作用。

    具体 CR 实施一定要有的放矢,那么在 CI 中运行生成的测试报告,质量报告就是那个“的”,连一个最基本的讨论<<基点>>都没有的话,只能说大家公司都是在走过场,说白了,你们根本就不是在 CR 。
    channingcheng
        36
    channingcheng  
       2021-01-04 14:22:07 +08:00
    @saulshao 可惜我们这边都是只有业务 owner 才 code review,大头兵没有资格 code review
    channingcheng
        37
    channingcheng  
       2021-01-04 14:37:57 +08:00
    @hantsy CI 和重构是两个问题呀
    hantsy
        38
    hantsy  
       2021-01-04 14:38:10 +08:00
    @channingcheng 你们那不是 CR 。只是普通的抽查工作情况(恰好拿代码挑刺说事),跟以前学校检查宿舍卫生一样,只有学工处干的事。
    hantsy
        39
    hantsy  
       2021-01-04 14:43:39 +08:00
    @channingcheng 不知道怎么跟你解释你的问题。。。
    Perry
        40
    Perry  
       2021-01-04 14:44:59 +08:00 via iPhone
    Unit Testing, Code Review, Integration Testing, Accessibility Testing, Stress Testing, Chaos Testing, Load Testing 等等这些不通过估计进不了大公司的 Production 。
    llllboy
        41
    llllboy  
       2021-01-04 15:09:53 +08:00
    就是个摆设
    DiverRD
        42
    DiverRD  
       2021-01-04 15:26:47 +08:00
    我一个版本改了 70 多个文件 组长看了 直接放弃 review
    Mirage09
        43
    Mirage09  
       2021-01-04 15:39:00 +08:00 via iPhone
    @Perry 我们组一堆 integration test 挂掉的 pipeline...
    USAA
        44
    USAA  
       2021-01-04 17:19:33 +08:00
    code review ? 这玩意不应该自己来弄吗
    flowerains
        45
    flowerains  
       2021-01-04 17:30:38 +08:00
    有时间才能互相 code review

    比如我们公司如果是压着点做需求,能按时按量把需求作完上线就不错了
    hantsy
        46
    hantsy  
       2021-01-04 17:55:04 +08:00
    @DiverRD 说明你们项目管理问题非常大。一个 PR 一般只包含一个 Feature,不应该太多,正常一般几个文件吧。如果是 Bug,可能可能几行代码,几个字符。
    hantsy
        47
    hantsy  
       2021-01-04 17:57:04 +08:00
    @llllboy
    @flowerains 跟时间没一点关系,这个和企业项目的工程文化有关。没写过测试,CR 就是摆设,没错。
    stirlingx
        48
    stirlingx  
       2021-01-04 18:08:15 +08:00
    @ferock 都 21 世纪了,鹅厂还用 svn
    onec
        49
    onec  
       2021-01-04 18:52:02 +08:00
    纯纯的摆设
    dfzj
        50
    dfzj  
       2021-01-04 20:08:54 +08:00
    在中型公司,项目分三个等级,只有 P1 等级才会 code review 。也就是是被定义为核心基础依赖的项目。
    一般的业务项目,功能测试过了就发布了。
    irytu
        51
    irytu  
       2021-01-04 20:30:14 +08:00
    我们公司 code review 很多都喜欢鸡蛋挑骨头 要说意义么 几乎可以忽略不计
    polyang
        52
    polyang  
    OP
       2021-01-04 22:06:33 +08:00
    @dfzj 感觉这个有道理。
    pangleon
        53
    pangleon  
       2021-01-04 22:06:50 +08:00
    楼上很多人没理解一点,CODE REVIEW 真正受益的是说人,他在给你介绍自己代码的时候也是重新梳理自己实现逻辑的流程,这个过程很能帮助他提高自己的思维甚至发现 BUG
    akira
        54
    akira  
       2021-01-04 22:24:16 +08:00
    不做 cr 的产品 确实是能跑能上线,但是后面出问题你要花费数以倍记的时间。 单元测试也好,压力测试也好,代码评审也好,这些都是为了降低风险引入的,做了后续出问题的可能性小,不做后续出问题的可能性大。

    出来混 迟早是要还的。
    yexiaoxing
        55
    yexiaoxing  
       2021-01-04 22:26:14 +08:00 via iPhone
    我们合规要求必须有人 review 才能 checkin 然后发 release 。
    lagoon
        56
    lagoon  
       2021-01-04 23:12:54 +08:00
    我觉得,先是设计好,然后开发好,再然后才是测试好。

    Code Review 肯定是好的,也是有用的。

    但许多中小公司的情况是,设计没办法好,开发没办法好,然后领导指望抓测试,指望 Code Review,就能稳住质量。
    本末倒置,只是安慰剂。

    能不能设计好,取决于领导。能不能开发好,取决于领导。
    领导不从自己身上找原因,寄希望于给开发人员找茬来稳住质量。



    因此,大部分公司,不会 Code Review,或者只能进行走形式的 Code Review 。
    hugo54
        57
    hugo54  
       2021-01-04 23:34:48 +08:00
    我所在的产品线的后端,都是 QA 来 CR 。 凡是非自主测试的代码,保准给你一行行 review 。
    vagranth
        58
    vagranth  
       2021-01-05 00:54:37 +08:00 via Android
    我司不光有 review,而且还要求写测试 case 。
    我曾经有一个大 patch 被要求拆分成 5 个 patch 慢慢 review 完的经历,要不是那个功能架构做的还可以,拆都拆不开。
    msg7086
        59
    msg7086  
       2021-01-05 01:23:34 +08:00 via Android
    我们是强制 code review,除了其他开发以外,team leader 也要 review 一次,没问题了才能合并。
    Leee
        60
    Leee  
       2021-01-05 08:32:30 +08:00 via Android
    @dayeye2006199 你们在广州吗?还要人吗😂
    cwliang
        61
    cwliang  
       2021-01-05 09:21:04 +08:00
    code review 很有必要,可以防止一些 anti-pattern 、性能冲击、维护性,虽然没时间看具体业务逻辑
    jorneyr
        62
    jorneyr  
       2021-01-05 09:29:47 +08:00
    理想很丰满,现实很骨感
    polyang
        63
    polyang  
    OP
       2021-01-05 10:09:09 +08:00
    @cwliang 我也觉得有必要,但首先是得给够时间,没时间的话,根本没办法做。
    leekafai
        64
    leekafai  
       2021-01-05 10:13:00 +08:00
    sr
    self review
    runliuv
        65
    runliuv  
       2021-01-05 10:49:38 +08:00
    有个锤子
    LemonK
        66
    LemonK  
       2021-01-05 12:42:01 +08:00
    @timedivision lint 和风格是两码事。比如有个前同事:他用他自己发明的标准划分为同类的变量,要求别人必须统一前后缀命名;多种语法都 ok 的逻辑,要求必须按他喜欢的那种写法;别人写 if 的地方他要求改成三目,别人写三目的地方他又要理论一下 if 更好。如果之前在这种代码孔乙己的身边工作,强调一下代码风格不重要还是有必要的。
    timedivision
        67
    timedivision  
       2021-01-05 13:51:04 +08:00
    @LemonK 风格不是一个人制定的,不管怎么说,代码这种二进制的东西,实现某个功能逻辑有很多种方法,但一定是有一个最优解的,尽管说简单的 if 判断扯不上什么最优解,但对于项目本身来说,统一的风格一定是利于维护的
    LemonK
        68
    LemonK  
       2021-01-05 15:00:01 +08:00
    @timedivision “写法一定有一个最优解”这句前提完全不认同。听起来就好像天下同一题目的作文只有一种标准写法一样。除了“一定不要这样写”的 Bad smell 共识之外,自由发挥的空间依然非常大。除非你只写 JAVA 。
    timedivision
        69
    timedivision  
       2021-01-05 15:57:30 +08:00
    @LemonK 不要断章取义啊,我说的是实现某个功能逻辑的方法一定有一个最优解,另外你拿作文和代码相比,这两者根本没有可比性
    mlbjay
        70
    mlbjay  
       2021-01-05 17:06:25 +08:00
    项目组有个老同事,技术很强,但是 CR 后 总我“建议”我改一些“可有可无”的东西,我也不敢反对,改咯。
    saulshao
        71
    saulshao  
       2021-01-05 18:13:33 +08:00
    @mlbjay 这其实就是代码 review 的价值。你可以尝试从中吸收对你有用的东西。
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   978 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 35ms · UTC 20:25 · PVG 04:25 · LAX 12:25 · JFK 15:25
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.