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

发起个讨论,你们公司有 code review 吗?

  •  
  •   zzzzzy · 2016-07-11 10:02:22 +08:00 · 15984 次点击
    这是一个创建于 3056 天前的主题,其中的信息可能已经有所发展或是发生改变。

    code review 好处很多,可以规范代码、传递知识和保证代码质量等,但是因为项目进度和其他因素,不一定保证 review 被有效执行。请教各路大神都是怎么做的?有没有啥套路、工具、方法之类的。

    108 条回复    2016-07-12 17:57:17 +08:00
    1  2  
    zhenjiachen
        1
    zhenjiachen  
       2016-07-11 10:11:11 +08:00   ❤️ 1
    技术经理:什么是 code review ?
    bigtan
        2
    bigtan  
       2016-07-11 10:12:11 +08:00
    完全不需要,自己写,自己用。
    chousb
        3
    chousb  
       2016-07-11 10:13:20 +08:00
    One on One Code Review.
    lution
        4
    lution  
       2016-07-11 10:13:45 +08:00
    不同的组不一样吧,反正我们组是必须 review 的。公司层面应该到了一定规模的公司很难规定这个东西。
    Yc1992
        5
    Yc1992  
       2016-07-11 10:13:55 +08:00
    方便某个人离职后其他人迅速交接
    tabris17
        6
    tabris17  
       2016-07-11 10:14:51 +08:00
    review by myself
    功能都来不及赶, review 个毛线
    repus911
        7
    repus911  
       2016-07-11 10:16:32 +08:00
    自己搭了个 gitlab 上线开 mr 有权限合的人去 review
    zzzzzy
        8
    zzzzzy  
    OP
       2016-07-11 10:18:39 +08:00
    @chousb 结对编程前期成本很高吧,还要看团队人员构成,我们这一个老员工, 3 个新来的小鲜肉,怎么破?
    zzzzzy
        9
    zzzzzy  
    OP
       2016-07-11 10:20:44 +08:00
    @tabris17 业务需求频繁、赶项目,可能确实要 review self
    mrwangrj
        10
    mrwangrj  
       2016-07-11 10:21:29 +08:00
    review 邮件 是展示工作量的方式。。。
    zzzzzy
        11
    zzzzzy  
    OP
       2016-07-11 10:21:36 +08:00
    @zhenjiachen 哈哈
    knightdf
        12
    knightdf  
       2016-07-11 10:22:19 +08:00
    自写自测自 review 自上线
    zzzzzy
        13
    zzzzzy  
    OP
       2016-07-11 10:22:59 +08:00
    @lution 基础的、功能性的组件确实要做 review ,请问你们怎么做的?
    zzzzzy
        14
    zzzzzy  
    OP
       2016-07-11 10:23:57 +08:00
    @Yc1992 你说的好像也有道理,哈哈
    murmur
        15
    murmur  
       2016-07-11 10:35:53 +08:00
    每个新人 的第一个模块要 review 的
    或者新系统的 每个人的 第一个模块
    后面看造化了
    lution
        16
    lution  
       2016-07-11 10:36:04 +08:00
    @zzzzzy 基本就是 7 楼说的那样
    hxtheone
        17
    hxtheone  
       2016-07-11 10:37:29 +08:00
    我们公司是所有的代码都要 review
    zhouxuchen
        18
    zhouxuchen  
       2016-07-11 10:46:36 +08:00 via iPhone
    刚进公司的时候负责人大概 review 了一个礼拜我的代码(当然到底有没有 review 我就不知道了)。后来一切就看一个人的命运和历史的进程了(
    zvving
        19
    zvving  
       2016-07-11 10:59:07 +08:00
    大家对 code reivew 好不重视啊。

    我们是用 gitlab - merge request 做的, develop 分支不能直接提交代码,全走 MR ,自己不能 merge 自己发的 MR 。
    好处就不说了。
    Phariel
        20
    Phariel  
       2016-07-11 11:00:37 +08:00 via iPhone
    Atlassian 的 Stash ,还有 Gerrit 都很好用
    aitaii
        21
    aitaii  
       2016-07-11 11:09:44 +08:00
    每周 4 下午有一个 code review 确实能规范公司的代码风格
    jmc891205
        22
    jmc891205  
       2016-07-11 11:10:34 +08:00
    我们用 code collaborator

    不过别人憋了两礼拜的代码一口气传上来让 review 的时候。。真的觉得不想去看
    quericy
        23
    quericy  
       2016-07-11 11:14:31 +08:00   ❤️ 1
    产品比开发都多,还来得及 review?
    sudoz
        24
    sudoz  
       2016-07-11 11:21:56 +08:00
    极个别的组有 review ,真的很少会审查代码,加班都来不及马丹的
    clino
        25
    clino  
       2016-07-11 11:24:20 +08:00
    我们这里不自觉做 code review,做不好 code review 的工程师设计质量和代码质量都比较差
    rekulas
        26
    rekulas  
       2016-07-11 11:28:22 +08:00
    500 强外企 用的 gerrit 来 view view 要经手好多人 最后的审核者还在国外流程有点慢
    rekulas
        27
    rekulas  
       2016-07-11 11:29:29 +08:00
    而且审核的也很严格 比如行尾多了一个空格。。。是过不了的
    repus911
        28
    repus911  
       2016-07-11 11:32:48 +08:00
    @jmc891205 没办法 有时候会在例会上说 今天我 review 代码 1 天 嗯 1 天
    odirus
        29
    odirus  
       2016-07-11 11:33:52 +08:00
    加班赶进度都来不及,还啥子 view 哦, just make it work

    公司人力资源多、钱多、注重技术,一般都会。
    menc
        30
    menc  
       2016-07-11 11:37:59 +08:00
    one on one ,用 gerrit 来做 review 。

    说不 review 的,都是没 review 过的, review 一下就知道了,注释, coding style 还有 bug ,问题茫茫多。

    代码不仅是写给自己看的,在公司里也是写给别人看的, review 不止有挑 bug ,也有保证自己代码别人能读懂的关系在。 code review 不仅会找出 bug ,也会要求修改格式,添加注释以增加代码可读性。

    在 V2 上看到有人推崇不写注释,要求代码自解释,来一个稍微大点的团队就知道,这根本就是狗屎。
    imswing
        31
    imswing  
       2016-07-11 11:39:07 +08:00
    技术经理:什么是 code review ?
    DevineRapier
        32
    DevineRapier  
       2016-07-11 11:47:03 +08:00
    不知道什么才算 review ,我们这写好了提交个 diff ,过了就可以提交。

    review 是,先提交,过后在一起回顾审核?

    感觉我们是 preview...
    cnly1987
        33
    cnly1987  
       2016-07-11 11:49:09 +08:00 via iPhone
    人多 就需要 review ,不然你敢 merge 其他人的分支吗
    expkzb
        34
    expkzb  
       2016-07-11 11:56:19 +08:00
    @DevineRapier 提交 diff 在 phabricator 里就算 review 。感觉这样挺好的
    monkeylyf
        35
    monkeylyf  
       2016-07-11 11:59:25 +08:00
    没有 最后代码库简直臭气熏天
    4everLoveU
        36
    4everLoveU  
       2016-07-11 12:00:49 +08:00
    RD 提测, QAreview
    nullp
        37
    nullp  
       2016-07-11 12:01:20 +08:00
    全靠自觉,,接收人骂爹
    hemingway
        38
    hemingway  
       2016-07-11 12:05:05 +08:00 via iPhone
    gefranks
        39
    gefranks  
       2016-07-11 12:19:02 +08:00
    据说是有 review 的,也有 reviewer 的名字,但是仍然不工作或者 bug 百出
    hahastudio
        40
    hahastudio  
       2016-07-11 12:31:22 +08:00
    kkzxak47
        41
    kkzxak47  
       2016-07-11 12:37:36 +08:00 via Android
    我们组加上负责人才 3 个人,名存实亡
    sc3263
        42
    sc3263  
       2016-07-11 12:39:23 +08:00
    之前的项目组有。
    svn 上新建 work 目录,上传原始代码,上传正式代码
    ->更新 redmine 上对应的案件,通知相关人员 review
    ->review 代码,有啥问题提一下,有啥 bug 修复一下,都在 redmine 上更新
    ->结束了上传到 trunk 目录,结束 redmine 上的案件
    代码质量确实高,不管是代码风格、注释,还是业务逻辑。写的人可能只考虑当前模块当前需求, review 的人会考虑到系统整体。毕竟 reviewer 对整个系统的了解,比我们不知道高到哪里去了。至于耗时什么的,我是觉得,整套流程跑起来之后,其实也不会多花很多时间,毕竟这边在 review 的时候,那边可以继续做别的需求的开发。
    qqzj
        43
    qqzj  
       2016-07-11 12:41:34 +08:00
    有,即使我改一行代码都有两个人来 review ,你得解释清楚为什么这么改,他们看得非常认真。有时还不止看改动,还把前面的代码也看下。顺便说下,他们都是工作 2 , 30 年的人,有时我都不好意思。
    czheo
        44
    czheo  
       2016-07-11 13:01:27 +08:00
    有,两人 review 之后才能 merge 到 master branch
    yangxiongwei
        45
    yangxiongwei  
       2016-07-11 13:10:46 +08:00
    用 phabricator ,强制 review
    easing
        46
    easing  
       2016-07-11 13:11:13 +08:00
    必须要有啊, code review 对质量的保证还是很有效果的
    SourceMan
        47
    SourceMan  
       2016-07-11 13:12:58 +08:00
    看造化吧,新功能都不够时间做
    shenxian
        48
    shenxian  
       2016-07-11 13:17:31 +08:00
    木有
    twoyuan
        49
    twoyuan  
       2016-07-11 13:37:01 +08:00
    @zzzzzy 小鲜肉表示刚开始老员工 review 比较好,熟悉后谁有时间谁 review 吧
    kimmykuang
        50
    kimmykuang  
       2016-07-11 14:24:32 +08:00
    gitlab 会发 commit 邮件,然后我偶尔会 review 下我们组的提交;每个项目版本提测前会组内 review
    broadliyn
        51
    broadliyn  
       2016-07-11 15:43:47 +08:00
    每次看到这类问题,我就想到那张图。
    要 copy - paste , good design 又不会给你加工资。
    人生没有时间给你重构。

    review 有个蛋用,功能都赶不及。

    young
        52
    young  
       2016-07-11 15:44:55 +08:00
    都是形式

    review 个蛋蛋
    zzzvvvxxxd
        53
    zzzvvvxxxd  
       2016-07-11 15:48:54 +08:00
    要的,你可以前期自己 review
    后期让实现功能的小鲜肉串讲一下,介绍一下代码的组织和设计的思路,你听一听就好了。
    Ixizi
        54
    Ixizi  
       2016-07-11 16:04:54 +08:00
    每次 commit 必须 review 。

    所有的队友必须一起 review 。
    Tonni
        55
    Tonni  
       2016-07-11 16:08:51 +08:00
    我们公司用的 GitHub Enterprise ,没个代码改动都要发送一个 Pull Request ,没个 Pull Request 必须要至少两个人 review 并且给予 👍 后才能 merge 到 master 里面去。
    daochouwangu
        56
    daochouwangu  
       2016-07-11 16:13:05 +08:00
    用 facebook 的 phabricator 来做 code review
    messense
        57
    messense  
       2016-07-11 16:42:45 +08:00
    Bitbucket PR + 自行开发的 CI 自动化测试、 Code Lint 自动化代码风格检查


    lovedebug
        58
    lovedebug  
       2016-07-11 17:05:15 +08:00
    每次提交修改前都要 review 吧~~
    公司用的 perforce, 和 Idea 以及 JIRA 结合 review code 还不错
    proudzhu
        59
    proudzhu  
       2016-07-11 17:07:12 +08:00
    @messense 道理我都懂,流程这么正规,为啥文件名这么随便。。。
    flydogs
        60
    flydogs  
       2016-07-11 17:07:44 +08:00
    有 review 率,有指摘率
    iphantom
        61
    iphantom  
       2016-07-11 17:28:10 +08:00
    用的 git 每次都需要 review 然后再合并
    我感觉 leader 只看看大概逻辑和写代码风格
    特别是有无性能特低的代码,有没有良好的缩进和注释习惯
    NovemberEleven
        62
    NovemberEleven  
       2016-07-11 17:28:30 +08:00
    外包行业,做项目的时间都不够,唉,还 review 。
    messense
        63
    messense  
       2016-07-11 17:39:37 +08:00
    @proudzhu 因为是 test fixture.....
    tianlang1989
        64
    tianlang1989  
       2016-07-11 17:48:44 +08:00
    有,个人觉得很重要
    我们公司时间也很紧,天天晚上加班
    然而 CR 流程一直保留
    clorts
        65
    clorts  
       2016-07-11 18:13:43 +08:00
    @Tonni github enterprise 价格多少?

    @Ixizi review 代码要看多久?
    @broadliyn review 是闲的蛋疼公司才干的么?功能都赶不及, review 个蛋啊:)
    @yangxiongwei 用这个多久了?是不是很烦躁呢?
    @repus911 review 这么麻烦啊?是一行行代码看过去么?都能看懂么?
    lightening
        66
    lightening  
       2016-07-11 18:15:02 +08:00
    有,必须有人 review 才能 merge 。
    repus911
        67
    repus911  
       2016-07-11 18:29:14 +08:00
    @clorts gitlab 我们 gitlab 用开源版 还没有特殊需求

    有的大功能比较麻烦,数据库看一遍, redis 看一遍,核心逻辑看一遍,兼容性过一遍,确认一遍。中间还有打回去修改,那边一个 sprint 10 天开发过来的 mr 怎么也得对上个 1 、 2 个小时。

    当然这种情况比较少见,一般 bugfix 10 分钟,小功能半个小时搞定,还有熟练工发过来的来基本也是 10 分钟内。

    别说我还真遇见过单元测试和 QA 没覆盖到的低级语法错误,嘿嘿嘿。
    yangxiongwei
        68
    yangxiongwei  
       2016-07-11 18:40:05 +08:00
    @clorts 习惯就好。代码质量就靠这个加 测试来保证了。

    而且这样是非常有必要的,前期花点功夫,后期维护起来会简单很多。
    kqz901002
        69
    kqz901002  
       2016-07-11 18:45:48 +08:00
    @sc3263 svn 可以和 review board 配合起来用,很方便
    shyrock
        70
    shyrock  
       2016-07-11 18:51:41 +08:00
    @chousb one in One?
    robert9484
        71
    robert9484  
       2016-07-11 18:52:47 +08:00 via iPhone
    没有啊
    DingSoung
        72
    DingSoung  
       2016-07-11 19:21:56 +08:00
    新功能没有。旧的代码,涉及底层公共的一般我自己写,让小弟看看久合了。剩下的就让测试妹子把关了。
    ourcubk
        73
    ourcubk  
       2016-07-11 20:19:53 +08:00
    代码->gerrit->自动编译 /UT/复杂度校验->review->review 通过触发自动编译生成 img->自动生成包->自动安装包->重启机器->启动完成后触发各种 case 跑自动化测试->代码 gerrit merge into git brunch->git brunch 自动 merge into brunch->终于他妈妈的完成了...........................................................................................................
    review 都是小事,重要的耗时间的都是 review 无关,却严格控制代码之类的其他工作...我们公司这样进去的代码,感觉还挺放心的...
    ourcubk
        74
    ourcubk  
       2016-07-11 20:20:46 +08:00
    git brunch 自动 merge into master...
    mahone3297
        75
    mahone3297  
       2016-07-11 21:00:57 +08:00
    再来一个话题,有写 test 吗?
    xwartz
        76
    xwartz  
       2016-07-11 21:28:09 +08:00
    代码烂成屎,入错坑了。。
    Totato5749
        77
    Totato5749  
       2016-07-11 21:30:45 +08:00
    @mahone3297 哈哈哈哈 没有
    sc3263
        78
    sc3263  
       2016-07-11 22:11:54 +08:00
    @kqz901002 是么?我试试。谢啦~

    其实讲道理,这东西,得项目组 leader 规定,强制执行才有效果。个人提出来,只会被认为工作量不饱和,闲着没事干。前几天在新项目组里提出来要搞一整套流程。然后被所有人反对了,理由是浪费时间。。。
    hantsy
        79
    hantsy  
       2016-07-11 22:18:55 +08:00
    @iphantom 你们不写测试吗》
    hantsy
        80
    hantsy  
       2016-07-11 22:24:16 +08:00
    基本上用 Github Flow 。。。 Fork , Branch, PR, Code Review

    结合 Circle CI 等,每个 Branch 都是自动运行测试,从建 PR 开始几乎团队所有人都会参与 Review ,特别与其他人有交叉的部分,@相关人,反复讨论修改。

    Upstream Master 在一些项目是稳定分支,执行自动部署。没 Code Review 谁敢直接部署?
    hantsy
        81
    hantsy  
       2016-07-11 22:26:38 +08:00
    @ourcubk Github 做 Code Review 不是更直接。。。
    metrue
        82
    metrue  
       2016-07-11 22:28:53 +08:00
    peer to peer review.
    hantsy
        83
    hantsy  
       2016-07-11 22:29:33 +08:00
    @clorts 偷懒的好理由。。。
    haoc
        84
    haoc  
       2016-07-12 06:29:41 +08:00
    之前小公司,基本上自己做一個 project ,別人也 review 不了。現在做的開源項目, 100+ contributors 。沒有 code review 和 CI 早就掛了吧。
    webdev
        85
    webdev  
       2016-07-12 08:40:08 +08:00 via iPhone
    只发生在找 BUG 的时候
    iphantom
        86
    iphantom  
       2016-07-12 08:59:41 +08:00
    @hantsy 单元测试么? 这个要写的,需要覆盖到。
    irisLi
        87
    irisLi  
       2016-07-12 09:27:06 +08:00
    我们公司每周都要 review 代码,我觉的这样我们大代码拿出来被大家看看才会知道自己哪里需要改进,这样我们才会有进步的啊!
    icegreen
        88
    icegreen  
       2016-07-12 09:36:19 +08:00
    每次 merge 必须 review
    yanyandenuonuo
        89
    yanyandenuonuo  
       2016-07-12 09:47:29 +08:00
    搭车问有哪些好的 review 的工具 :)
    xiaowangge
        90
    xiaowangge  
       2016-07-12 10:04:25 +08:00
    没有。

    上周刚从 SVN 切换 到 Git 。

    后端开发至少 7 人,前端开发至少 9 人。
    beew
        91
    beew  
       2016-07-12 10:08:53 +08:00
    开发完成后自己提交一个 MR , leader 会 review ,通过了再 merge ,否则打回改到符合要求。项目稳定之后这么执行其实很好的,整套流程配合起来很顺畅
    salltm
        92
    salltm  
       2016-07-12 10:37:08 +08:00
    这么说吧。 外企基本上都有 review 的, 而且是算工作量的。 Reviewer 需要承担责任。 所以一般会比较认真 , 貌似看起来比较费时间,但是带来的产品质量提升很有效。 也少了给项目埋地雷的机会。。

    国内很多公司急功近利,不重视这个, 总归是要走弯路的, 修不完的 bug 。。
    Froyo9
        93
    Froyo9  
       2016-07-12 10:37:17 +08:00
    都说会有,但是真正做起来的很少,哎
    andyL
        94
    andyL  
       2016-07-12 10:38:02 +08:00
    我们老大会看每次提交,一些修改会做 review ,也会对这些修改进行测试。我觉得 review 是必须的,但是 reviewer 需要是有经验有实力的 coder ,要对整个系统很熟悉。经常会给我们指出错误和提出优化意见,很棒。
    hantsy
        95
    hantsy  
       2016-07-12 11:47:08 +08:00
    @iphantom 看来国内还是有那么 10%的公司重视软件开发质量的。。。
    Just1n
        96
    Just1n  
       2016-07-12 12:07:53 +08:00
    我们公司都是每个组组员之间互相 Review 。
    ayiis
        97
    ayiis  
       2016-07-12 12:15:58 +08:00
    有,我负责 review ,我最喜欢喷人了
    liuzhen
        98
    liuzhen  
       2016-07-12 12:17:19 +08:00
    不定期 review
    cjyang1128
        99
    cjyang1128  
       2016-07-12 12:23:24 +08:00
    review 就跟改作业一样,老师不帮你改作业,你难道不慌嘛
    hitmanx
        100
    hitmanx  
       2016-07-12 13:01:02 +08:00
    有啊,在外企。经常不到10行代码 review 好几个来回耗时几天,各种挑毛病,实在没大毛病还得挑变量名啊, trailing spaces 啊之类的,痛苦得扭来扭去。
    1  2  
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   实用小工具   ·   943 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 29ms · UTC 20:41 · PVG 04:41 · LAX 12:41 · JFK 15:41
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.