谈谈 code review？

[sockpuppet9527]

• 优点：保证代码质量，保证工程代码风格一致。
• 缺点：过度 review 导致项目效率低下，遇到不专业的 review 只想喷人。

最近就遇到个烦人的，改来改去，就写个模块的接口，反反复复改，想到喊我改到哪。 比如一个函数：

int xxxx_init(CTX * ctx,A *a){
	if (xxx){
    	return rc;
    }
    xxxx // 实际逻辑代码段
    return rc;
}

一般来说，项目风格就是这样的，先检查 ctx 啥的，然后如果实际逻辑，最后返回调用状态。 这个方法能给我提 3 个 comments 。

1. it is simple memcpy... do we really need all the checks below?
2. i guess this function should return，remove A * a 。
3. is it documented on API level? (实际逻辑代码段上一顿 bb)

看到这个我都不想回也不再改了，想问你调用 CTX 是有状态的，你调用函数前不需要 check ？其二本身方法逻辑就是大页来分配 A，名字也叫 xxx_init，我还返回个毛球啊。

搞得真想跑路。

[fengjianxinghun]

code review 确实 sb 。

[fengjianxinghun]

深层问题从来看不出来，只能在一些格式风格上吹毛求疵以符合 code review kpi 。

[Salvation]

核心问题不是 cr，而是 cr 的执行方式，执行者。

[shenlanAZ]

感觉你们的 code review 少了连带责任，如果 reviewer 的建议导致业务出了问题 reviewer 需要承担一定的责任。

不过我觉得 code review 最大的好处就是减少项目的烂代码，有些实习生或者代码写的烂的人 ，提交上去的代码能让你粗口半小时，code review 在解决问题的同时 还能帮助他怎么才能做到更好。

[MarshallMathers]

哪一家啊 lz??

[coderluan]

你们这是随时聊天或者发邮件 review 的? 建议还是开会 review 吧, 做好记录, 效率提升很明显的, 起码不会总出现一样问题浪费时间.

[luckyrayyy]

code review 不能背这个锅吧，要喷也应该喷同事 xxx

[sockpuppet9527]

@Salvation #3 非 maintainer 的 review，往往让人很迷惑
@coderluan #6 有专门的 review system

[sockpuppet9527]

@luckyrayyy #7 差不多是这个意思。缺点特指：过度 review，咬文嚼字。

[securityCoding]

code review 需要很深的功力 ,需要从代码和业务视角来审视代码

[GoLand]

你这个是 reviewer 的问题，典型的爱钻牛角尖，不会 review 来 review 代码简直比重写一百遍还难受，建议以后你 review 的时候，给他也来一下类似的操作，每块逻辑提几个 commet，他就知道有多难受了，后面应该就会改进。

[wutiantong]

怼呀，所以是个人都是 review 另一个人的代码么？肯定不是啊，你凭啥来 review 我的代码，你水平够么？
就这 3 点 bb，你来改，我看看你要改成啥样子。

[coderluan]

@sockpuppet9527 系统不能取代开会的, 比如这种再开会的时候当着大家的面提出来, 之后对方肯定会收敛不少, 当然你也可以在项目会议上说一下, 你自己憋着和网友吐槽都没啥用, 你都自己想跑了, 还有啥顾忌, 开会当场撕他啊.

[wutiantong]

@wutiantong 都是==》都能

[sockpuppet9527]

@coderluan #13
就拿第二点来说，假如需要你把方法名改成 xxxxx_create，然后把参数中的指针改为返回，这点我就很难反驳他，因为这两种方式是一样的(在本项目中)。这种事情你怎么和他扯的清楚？
我觉得开会讲这种问题，没个头，我目前来说，只能妥协。之后这块谁爱改谁改。约等于掉了一次坑。

[sockpuppet9527]

@wutiantong #12 是这样的，我个人始终认为应该就几个 maintainer 来 review 就好了，现在互相 review （而且国家还不同）搞得效率极低。

[gadsavesme]

我们之前都是每周挑个一两小时开会组里人一起 review，规范大家定，但是得遵守。

[coderluan]

@sockpuppet9527 开会不是让你讲具体问题, 而且去约定 review 的范围, 没实际影响的内容就别提, 由 maintainer 自己定, 但是你说你们国家不同, 这个可能才是说这个的障碍, 反正你说这个我第一时间就感觉对方是印度人, 算是个人歧视吧, 但是我的印度同事确实也这样, 也确实没办法说.

[xuanbg]

代码评审不必责备求全，可以逐步推进。
第一步要解决的是代码风格问题，统一的代码风格评审起来才能事半功倍。
第二步是代码规范问题，Java 的话按阿里的规约取舍一下就好。规范执行到位，bug 能少一大半。
第三步是代码结构问题，编程最大的问题不是写错了代码，而是代码没写对地方。这个问题包括但不限于：代码冗余、过大的类和方法、方法有太多的参数、项目结构混乱或根本没有结构……说白了就是没有任何封装或者错误的封装。

以上做好了，代码基本也就没啥毛病了

[hakono]

一个 21 岁入职的年轻人做 code review （有前职经验），进来就对着我的代码给了十来条 comment，然后我觉得有意义的地方改了，没问题的地方没改，回复了一下为什么不改。然后对方直接开始了和我你来我往几千字以上的文字交锋

我因为日语非母语，对方是日本人，还一大段日语打下来从不加标点（就连日本人同事都只喊他这日语受不了），和他互相交锋浪费了 2 天时间，最后代码没合并，项目没进展他还不依不饶，把项目开始至今为止的经纬和为什么这么做都给他解释了一遍，怎么解释都不听。
最后气得我直接爆粗，结果对方还跑去组长那让组长做定夺。自然了解项目始末的组长认为我的写法比较好，最后那人才罢休。感觉这两天时间是彻底浪费掉了

有些人做 code review 是真的纯属浪费时间