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

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

  •  
  •   z0z · 2017-12-01 10:59:27 +08:00 · 6087 次点击
    这是一个创建于 2552 天前的主题,其中的信息可能已经有所发展或是发生改变。
    待过的公司从来没有这一高大上的环节。
    经历过的来给说说吧,开开眼界。
    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 厉害了
    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   ·   实用小工具   ·   1068 人在线   最高记录 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 26ms · UTC 22:18 · PVG 06:18 · LAX 14:18 · JFK 17:18
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.