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

codereview

高大上

眼界

说说

26 replies • 2017-12-01 19:44:51 +08:00

kuro1

Dec 1, 2017

1

不用去公司，找个开源项目 pr 一下就懂了

Phariel

Dec 1, 2017

2

Gerrit + CI vote

sneezry

Dec 1, 2017

3

就是一群人上去围观你的代码，然后提出任何大大小小的问题，注释也不会放过。

Just1n

Dec 1, 2017

4

我厂一个 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

Dec 1, 2017

5

给你看个 Apache Spark 的 code review ( https://github.com/apache/spark/pull/19717)，一堆人在上面 review，comment，然后 trigger test，最后 committer 会 LGTM 然后 merge。通常对于大的 PR，review 可能会有很多轮。

wangshuCoding

Dec 1, 2017

6

@sneezry 厉害了

whx20202

Dec 1, 2017

7

@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

Dec 1, 2017

8

打开 IntelliJ, Command+9, 一个一个 commit 来...

tamlok

Dec 1, 2017 via Android

9

略

assad

Dec 1, 2017

10

@Just1n 理想化啊，当需求不停砸过来的时候，有时候没法严格按照这个来吧

clino

Dec 1, 2017 via Android

11

去 android 的 gerrit 观摩下就知道了

sangmong

Dec 1, 2017

12

一行一行给老板讲干什么用的，老板觉得可以了然后 commit

Just1n

Dec 1, 2017

13

@assad #10 也是严格遵守这样的流程以确保产品和代码质量。

z0z

OP

Dec 1, 2017

14

@kuro1
@Phariel 好，我去找找看。
@sneezry 简单，粗俗，易懂。

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

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

@zhidian 表示不认识 IntelliJ

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

@clino 尝试观摩去

@sangmong 那就是说每个人都要讲一下自己的是吧
@Just1n 虽然不懂，但是坚持认为确定流程后就要遵守。因为流程就是从经验教训中总结出来的，可以减少不必要的重复意外。