你们的 codereview 是怎么样的流程? - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
请不要在回答技术问题时复制粘贴 AI 生成的内容
z0z
V2EX    程序员

你们的 codereview 是怎么样的流程?

  •  
  •   z0z 2017-12-01 10:59:27 +08:00 6469 次点击
    这是一个创建于 2875 天前的主题,其中的信息可能已经有所发展或是发生改变。
    待过的公司从来没有这一高大上的环节。
    经历过的来给说说吧,开开眼界。
    26 条回复    2017-12-01 19:44:51 +08:00
    kuro1
        1
    kuro1  
       2017-12-01 11:01:21 +08:00
    不用去公司,找个开源项目 pr 一下就懂了
    Phariel
        2
    Phariel  
       2017-12-01 11:02:10 +08:00
    Gerrit + CI vote
    sneezry
        3
    sneezry  
       2017-12-01 11:07:59 +08:00
    就是一群人上去围观你的代码,然后提出任何大大小小的问题,注释也不会放过。

    Just1n
        4
    Just1n  
       2017-12-01 11:13:12 +08:00
    我厂一个 Defect 类型的 Work Item 大致流程是这样的:
    客户提一个 incident, 产品确认是否是 defect,如果是就 escalate 并创建一个 work item ->
    自动分配系统把这个 WI 分配到有空闲工作时间的 Developer 手上,这个 WI 就在 Developer 的工作队列中 ->
    Developer 拿到这个 WI 开始调查,确认 Root Cause ->
    写 UT ->
    写功能( fix )代码 ->
    提交代码到 DAT ( Distributed Automated Testing ) Server 上跑一次全部的 UT (此期间 DAT 会自动根据提交的代码产生一个 Build 给后续的产品做测试) ->
    DAT 通过之后 WI 就会到 Code Reviewer 那边,Reviewer 开始审查代码 ->
    审查通过之后 WI 到产品那边,产品会用上上一步生成的 build 做 Functional Review ->
    Functional Review 通过之后代码就可以 Checkin, 一个 WI 结束。

    这个过程的话,Developer 所花的时间大概就只有 Investigation 和 Coding 的时间,其他基本都是自动化。
    jerryshao1984
        5
    jerryshao1984  
       2017-12-01 11:25:42 +08:00
    给你看个 Apache Spark 的 code review ( https://github.com/apache/spark/pull/19717),一堆人在上面 review,comment,然后 trigger test,最后 committer 会 LGTM 然后 merge。通常对于大的 PR,review 可能会有很多轮。
    wangshuCoding
        6
    wangshuCoding  
       2017-12-01 11:43:58 +08:00
    @sneezry 厉害了
    tr>
    whx20202
        7
    whx20202  
       2017-12-01 11:54:12 +08:00
    @jerryshao1984 请问一下怎么在 github 的页面里面,插入代码?
    比如你给出来的链接中的:
    core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
    Show outdated resource-managers/kubernetes/core/pom.xml
    Show outdated .../kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
    Show outdated ...etes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/Client.scala
    Show outdated ...etes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/Client.scala
    ...src/main/scala/org/apache/spark/deploy/k8s/submit/DriverConfigurationStepsOrchestrator.
    zhidian
        8
    zhidian  
       2017-12-01 12:10:26 +08:00
    打开 IntelliJ, Command+9, 一个一个 commit 来...
    tamlok
        9
    tamlok  
       2017-12-01 12:11:09 +08:00 via Android
    assad
        10
    assad  
       2017-12-01 12:15:00 +08:00
    @Just1n 理想化啊,当需求不停砸过来的时候,有时候没法严格按照这个来吧
    clino
        11
    clino  
       2017-12-01 12:15:30 +08:00 via Android
    去 android 的 gerrit 观摩下就知道了
    sangmong
        12
    sangmong  
       2017-12-01 12:17:18 +08:00
    一行一行给老板讲干什么用的,老板觉得可以了然后 commit
    Just1n
        13
    Just1n  
       2017-12-01 14:41:06 +08:00
    @assad #10 也是严格遵守这样的流程以确保产品和代码质量。
    z0z
        14
    z0z  
    OP
       2017-12-01 14:59:26 +08:00
    @kuro1
    @Phariel 好,我去找找看。
    @sneezry 简单,粗俗,易懂。

    @Just1n 我一个单词都没看懂啊。这说明,我们太不正规了,对,太不正规了。

    @jerryshao1984 404,得翻墙吗
    @whx20202 依然不懂你在说啥

    @zhidian 表示不认识 IntelliJ

    @tamlok 你说啥,你们也没有?

    @clino 尝试观摩去

    @sangmong 那就是说每个人都要讲一下自己的是吧
    @Just1n 虽然不懂,但是坚持认为确定流程后就要遵守。因为流程就是从经验教训中总结出来的,可以减少不必要的重复意外。
    rannnn
        15
    rannnn  
       2017-12-01 14:59:55 +08:00
    https://bitbucket.org/atlassian/atlaskit/pull-requests/4572/ak-3744-fix-component-fix-icon-gradients/diff
    大概就是这样的。

    不过我们公司内部用的是 Bitbucket Server 版,diff 要更加干净一些,而且有文件树,还可以只看某个 commit 的 diff
    大概类似这样:
    http://atlassian.wpengine.netdna-cdn.com/wp-content/uploads/stash-pull-request-diff.jpg
    wujunze
        16
    wujunze  
       2017-12-01 15:19:15 +08:00
    @Just1n #13 应该是大厂吧 666
    hahastudio
        17
    hahastudio  
       2017-12-01 15:37:58 +08:00
    sangmong
        18
    sangmong  
       2017-12-01 17:00:02 +08:00
    @z0z 提到服务器上的每行代码都要各个组的老大 review,包括注释,这样可以确保像虾米音乐乞丐版会员的情况不会出现,23333
    aksoft
        19
    aksoft  
       2017-12-01 17:03:10 +08:00
    是骡子是马拉出来溜溜
    tonghuashuai
        20
    tonghuashuai  
       2017-12-01 17:05:01 +08:00
    一撮人找一个有投影的会议室然后围观一个人的代码
    Bazingawang
        21
    Bazingawang  
       2017-12-01 18:08:22 +08:00
    要不要试试 Coding 的 Code Review 功能?
    ![图片]( https://dn-coding-net-production-pp.qbox.me/2816fea1-8ad3-455a-bc27-8432c18e44bf.png)
    在提交 MR 后,评审者对代码进行评审,提供逐行对比、逐行评论等功能,可以适应大部分的代码 Review 场景。
    ![图片]( https://dn-coding-net-production-pp.qbox.me/78f474e9-4f5f-4a46-9ebc-ddb59095c985.png)
    wxkvEX
        22
    wxkvEX  
       2017-12-01 18:40:25 +08:00
    我们的 codereview 就是不进行 codereview
    自信这条很多公司都中。
    xrlin
        23
    xrlin  
       2017-12-01 18:44:17 +08:00 via iPhone
    @wxkvEX 握爪,单元测试都很少写,但感觉这样很不好。
    Him
        24
    Him  
       2017-12-01 18:56:53 +08:00
    @sneezry reviewer 什么时候去 review 代码?随时?
    zhx1991
        25
    zhx1991  
       2017-12-01 19:01:02 +08:00
    就是找一堆人去会议室一起围观代码
    xiqingongzi
        26
    xiqingongzi  
       2017-12-01 19:44:51 +08:00
    @sneezry #3 这种范围模糊是用什么软件做的呢?求教
    关于     帮助文档     自助推广系统         API     FAQ     Solana     3552 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 29ms UTC 00:18 PVG 08:18 LAX 17:18 JFK 20:18
    Do have faith in what you're doing.
    ubao msn snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86