大家都怎么做code review....

乱码

刚才给新来的consultant作code review，指出几个很严重的performance的问题（小小不然的我都算了），刚开始说他还比较能接受，后来说着说着脸越拉越长，到最后说：我要去赶汽车，我明天问Matt(另外一个snr consultant)怎么做吧...

我基本上忽略他的反应，code review就是质量把关，管你是谁...让我来做，就要符合我的标准，更何况是严重的错误。

自我感觉我算是标准放的很宽的了，原来有同事做frontend code review，能让人起每个browser做兼容性的测试.

大家都是怎么做code review的？照顾同事的面子？还是一丝不苟保证code质量？

netstat

这种鸟几次这样就开掉算了

da_wang

干吗当面做，用code review软件阿

uowzd01

never officially did code review before, although TFS asks for code reviewer, I will just pass a random name

napolian

lz能介绍下你的review流程嘛？

kr2000

代码重用性和优美程度就懒的管了
性能还是很重要的

你的review结果是不是跟对方拿不拿的到钱有关啊

收路费

你一味说不好的 当然比较难让人接受
听以前前辈教导的 要采取所谓三文治的方式
先夸一个 再批评一点 最后再夸一个

atransformer

混不到坑的萝卜

贴code出来，让我们集体review一下你的review，嘿嘿，看你是不是也太鸡蛋里挑骨头了。

slateblue

原帖由 乱码 于 2011-9-13 17:04 发表
刚才给新来的consultant作code review，指出几个很严重的performance的问题（小小不然的我都算了），刚开始说他还比较能接受，后来说着说着脸越拉越长，到最后说：我要去赶汽车，我明天问Matt(另外一个snr consultant)怎么做吧...

...

是正确的，就要坚持原则。team就是要共同进步的。

bffbffbff

原帖由 收路费 于 2011-9-13 21:22 发表
你一味说不好的 当然比较难让人接受
听以前前辈教导的 要采取所谓三文治的方式
先夸一个 再批评一点 最后再夸一个

同意这个， 不能一位直接批评，大家都是被请来做事情的，谁也不欠谁的，所以最好适当的鼓励然后稍微温和的批判一下

这tmd是consultant么，我还以为consultant对公司的code 进行review呢

atransformer

原帖由 bffbffbff 于 2011-9-13 22:03 发表


同意这个， 不能一位直接批评，大家都是被请来做事情的，谁也不欠谁的，所以最好适当的鼓励然后稍微温和的批判一下

这tmd是consultant么，我还以为consultant对公司的code 进行review呢

是。 consultant 更能混饭吃。 我原来的可爱的同事，连.net 1.0 和 2.0 啥区别都不知道的大哥居然在一个很大的咨询公司混了快两年，这不，最近又跳到了另一家。

fly050505

乱兄，
没有必要这么搞，毕竟不是自己的公司。
性能再差，能差多少？
你自己把问题记在心里，有问题时，再修改就是了。
当然了，如果是外包公司，自然要严格一点了。

和你分享一下，我的经验：
我以前管理团队时，先让每个人提供一段sample,
整理出来一个bad example，和good　ｅｘａｍｐｌｅ。经常更新这个ｄｏｃ．
ｃｏｄｅ　ｒｅｖｉｅｗ时，盲选。ｃｏｐｙ大家。
这样，有了标尺，谁都不会说啥。
好就是好，差就是差。
不会结下私人恩怨。

这样，作为ｌｅａｄｅｒ，你基本不用干活，还能提高自己。
我个人认为，ｃｏｄｅ　ｒｅｖｉｅｗ的作用不是找毛病，是为了提高自己。

具体操作细节，可以私下交流。

祝好运。

jn1216

如果有错，当然要改。要不然，出错了，你是reviewer，也有你好看的。

fly050505

原帖由 atransformer 于 2011-9-13 22:09 发表


是。 consultant 更能混饭吃。 我原来的可爱的同事，连.net 1.0 和 2.0 啥区别都不知道的大哥居然在一个很大的咨询公司混了快两年，这不，最近又跳到了另一家。

哈哈，用ｃｏｎｓｕｌｔａｎｔ我更有经验，
目的就是尽量让他们处理问题，你来吸收经验。

一开始，就要让他们提供很多
ｃｏｄｅ　ｒｅｖｉｅｗ　ｄｏｃ，
ｎａｍｉｎｇ　ｃｏｎｖｅｎｔｉｏｎ　ｄｏｃ．．．

多吸收其精华嘛。

greed

我一般也就睁只眼闭只眼。这里不比国内，动不动人家就会感到stress，以此为由休病假的也时有耳闻。大问题的话我会说点好话，抱怨一下系统啦工具啦，再说说我的想法。甚至会来个座谈会大家就某个具体subject胡侃一通。人多有人多的好处，插科打诨加上集思广益。这里的水的人太多，认真的人伤不起啊。以前遇到几次，it 科班出身，混了好几年了，还要从OO的基础说起，有木有啊有木有？

linkspeed

我喜欢乱码这样的TL，啥时候招人招呼一声。

righttang

我一般不会把话说死，一般委婉一点说，如果是我，我会怎么做，你如果觉得这样好，那我也同意。

更或者，我会以提问的方式问他，你这样写，会不会有性能上的问题呢？我也不清楚，我们讨论讨论。让他自己想想，你别急着下论断。

如果是坚决要改的话，我一般也会说，我strongly recommend你不要这样写。。。。

点到为止，如果别人不问，你不用去说，你应该怎么怎么弄。毕竟每个人都有自己一套做事的方法，看重结果的同时，具体过程不用太过讲究。

o2h2o

照顾面子还是很重要的
委婉一点
毕竟不我们都不年轻了
做人要圆滑一点

你不见的 唐太宗这样的明君都叫嚷的要杀魏征 N 次

我老婆公司里面有个韩国人，暴勤奋
但是
就是做事很较真
一点不给面子


当然如果是很夸张的错误
也不用管面子了

[ 本帖最后由 o2h2o 于 2011-9-14 10:26 编辑 ]

乱码

谢谢大家阿，没想到这么多的回复，昨天下班前发的帖子，晚上忙家里的事没来的急上网，现在一起回一下。

@netstat
他是consultant，我们开不了他的，现在换人都太晚了,不过下次我们开发，估计他再来的可能性很小了。

@da_wang
从来没用过code review的软件，从来都是人工作，我猜它软件能找出小问题，但结构性的大问题它可能就无能为力了，correct me if I'm wrong.

@uowzd01
what is "random name"? bob blah...?

@napolian
也没什么具体的流程，tfs要求checkin的时候，policy要求有code reviewer，我们一般找team的人来做。先brief一下task的要求（在PBI的description有写），然后比较一个文件的改动，并说明理由。

如果review满意改动，code就可以checkin，这时候实施者跟code reviewer共同承担code的责任，一般reviewer可能责任更多些，因为一般都是snr或更熟悉系统的人来做review.

@kr2000
code review跟他的薪水没关系，他怎么都能拿到钱, 即使我打他一顿他也能拿到钱

@变形金刚
consultant这个行业水很深，但整体素质要比一般公司的dev team水平高些，但不排除有些混混在里面，我觉得水平太凹了肯定混的也不舒服。

@萝卜
他的小问题例如代码级别的我都不挑，不然没完没了，对他我只捡架构性的错误。

@slateblue && jn1216
嗯，我个人的观点，这是code review的目的之一.

@bffbffbff && 收路费 && righttang && greed && o2h2o
我个人觉得平时做review是挺注意措词，好话先说，坏话都是跟在but后面，而且是建议性的，但昨天他上来就给我看一个根本不需要的store proc，让我好话都没地方说，直接就来建议了，可能的确是有这个问题。

不过他是个愤青，平时正常说话都能翻脸那种（不过不是跟我），水平也是属于left outer join,inetmgr不知道是啥的....我觉得即使夸的他跟花一样，他也真不一定能领我的情。

@linkspeed
又让你失望了，我的确不是什么tl，我喜欢管闲事，让别人比较烦

@fly050505
谢谢你这么详尽的建议阿！！

我觉得你跟我性格差不多，都是比较平和的那种，跟人交往不带什么进攻性。

不过business付我钱来deliver有质量的产品，我的确不能随便让烂code checkin进去，尤其是架构性错误的那种，否则以后很难维护。

他这次最严重的错误是很多database的chatty talk，如果有1万条记录，他的approach就要跟query database 1万次，真的很不能接受。

我们有自己的coding standard，没什么特殊的，都是大路的要求，这种错误是一个普通的developer都不能犯的，更不要说是consultant.

其实我早就不把他当consultant了，就当一个jnr developer来用，让他写code的时间，我写10个都出来了,但这都不是主要问题。

最重要的他干活心不在焉，我跟其他snr consultant/BA往来的技术email/要求都是抄送他的，需要他注意的地方我都把他的名字highlight一下，结果他错误照犯...

另一个问题是他对修改现有的code很没有信心，动一个class都心惊胆颤，生怕碰到其他不该碰得，我说你不用害怕，花些时间做调查，看看其他地方是不是还用这个class，如果不用，你可以大胆的改动...

现在vs的工具也很发达，如果一旦break什么东西，肯定可以fix好的，整个solution在local上就是自己的，要有信心来操纵，他恰恰很缺乏这种信心，这个也很要命。

在客户这边干活，就少上些linkedin的那类东西，即使上，也没关系，不要瞎吹，什么mentoring jnr，tech vp...啥的，当大家都不懂google呢？

最后一个他没有bigger picture,也不想有，一些有关联的东西他连看都不看，这也是导致他犯错的直接原因之一.

我们的结构决定了做code review主要是来保证产品质量，提高可能有，但很少。

私人恩怨我的确不怎么在乎，对有些人，我不愿意去social,但对另外些人，我更愿意表现得友好些，足迹有我几个原来的同事，我做事什么风格他们很清楚。

乱码

原帖由 o2h2o 于 2011-9-14 10:17 发表
照顾面子还是很重要的
委婉一点
毕竟不我们都不年轻了
做人要圆滑一点

你不见的 唐太宗这样的明君都叫嚷的要杀魏征 N 次

我老婆公司里面有个韩国人，暴勤奋
但是
就是做事很较真
一点不给面子
搞的全公司没一个人喜欢她
失去了很多晋升的机会

当然如果是很夸张的错误
也不用管面子了

嗯，我觉得你这点作的很好，我有时候是有点直，但咱们这个行业比我直的海了去了，我自己觉得算特一般的那种.

huazhb

怎么会有这种错误? 太可怕了. 这种系统一个月以后客户就要抱怨慢了

Fernando

We need more 乱码 to control the quality of the code.
I can see rubbish code everywhere. Really hate it.

Dan.and.Andy

打个叉，这人是年轻的三哥或者帮哥拉袋屎的吧 ？

乱码

原帖由 huazhb 于 2011-9-14 10:38 发表
怎么会有这种错误? 太可怕了. 这种系统一个月以后客户就要抱怨慢了

幸好这是给3rd party提供feeds的东西，一天就run一次，query也不是live database，否则真不能给他做.

乱码

原帖由 Fernando 于 2011-9-14 10:41 发表
We need more 乱码 to control the quality of the code.
I can see rubbish code everywhere. Really hate it.

谢谢阿，it's idea of tech QA.

cloud226

原帖由 收路费 于 2011-9-13 21:22 发表
你一味说不好的 当然比较难让人接受
听以前前辈教导的 要采取所谓三文治的方式
先夸一个 再批评一点 最后再夸一个

同意

乱码兄应该还是保留了比较多的中国人的直来直去 鬼佬一般说话会比较委婉的 更不用说在这种关乎工作能力的问题上了

不过其实也没有什么大不了的 如果一直做好好先生 反而让人觉得没有个性 好恶不分 更不好

典

支持，
大家都不容易，要干就好好干，否则让位子。
这种烂Code, 一定要挡住。

乱码

原帖由 Dan.and.Andy 于 2011-9-14 10:45 发表
打个叉，这人是年轻的三哥或者帮哥拉袋屎的吧 ？

haha,这次不是，那个三哥是一个很好的team player，我们现在都很喜欢他

这个consultant是米国LA来的，不清楚国籍，香港人，曾经在香港工作一年....

我有时候特别想问他为什么不在LA继续发展，毕竟加州是做技术的大本营...

他可能真的做过tech vp，但性格决定命运....不管技术如何，控制好自己的情绪是做management的关键.

乱码

原帖由 cloud226 于 2011-9-14 11:01 发表


同意

乱码兄应该还是保留了比较多的中国人的直来直去 鬼佬一般说话会比较委婉的 更不用说在这种关乎工作能力的问题上了

不过其实也没有什么大不了的 如果一直做好好先生 反而让人觉得没有个性 好恶不分 更不好

可能现在我更倾向于get straight to the point/get things done, 我觉得我原来更注重的是委婉，但执行力度不够...

感觉适度的pushy效果更好些.