Topic: 也说说 code review @渥太华华人论坛:渥太华枫下论坛 The Rolia Forum of Ottawa

工作学习 / 事业工作 / 也说说 code review -xmlhttprequest(build5381); 2021-12-4 {767} (#14157128@0) +2
几年前因为一个项目工期紧，公司雇了一个合同工，sr。
第一个 commit 就很粗糙，主要就两个 methods，居然有一半 code 是一样的，就不能提炼出来放在一个独立的 method 里共享，完全是不过脑 copy-paste。
打回去一两次，改了，之间也没什么冲突。
第三个礼拜突然秒 quit，然后有人告诉我，他在 exit interview 时说是对我有意见。
其实我们都知道很大概率是他拿到了个更高的 offer 随便找个借口（一共两三个礼拜他没什么机会和其他人打交道）---- 但我还得截屏所有 code review 记录 + slack 记录，发给我老板。我老板肯定了我的所有 code review comments / communication 没有任何问题，完全 professional。我还得知会 hr 我和我老板的 review，以及老板也认为没问题。
hr 倒是云淡风轻地秒回 email 说没啥的 it happens --- 多半 hr 也知道他只是随便找借口。
一直心有余悸。

歪个楼: 原来工作很好找么，这么个态度的没三周就又找到更高的了? -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-4 (#14157138@0)

我们当时感觉极大概率是他另外一个更高的 offer 到了。不觉得他是现找的。 -xmlhttprequest(build5381); 2021-12-4 (#14157150@0) +1

也许我太偏颇和狭隘。西方公司文化缺乏公平和正义，从CEO到小兵，大家都是得过且过，捞一把就走的心理。这点对我来说是个很大的困境，适应不好。 -remdesivir(larevo); 2021-12-4 (#14157139@0) +3

嗯。外国大锅饭。缺乏的评价过了 -newrover(漫游); 2021-12-4 (#14157143@0)

我去。这哥们水平有点惨。貌似这毛病我都能看出来。人品更惨，你实际上帮助他提高了，他反咬一口。😂😂 -newrover(漫游); 2021-12-4 (#14157140@0) +4
很多人是因为钱多才做合同工，不是因为水平高。合同工里水平差的比比皆是，遇到个态度认真的就不错了，不能期望水平有多高。 -househot(花开花落); 2021-12-4 (#14157146@0) +1

没期望他水平有多高，也理解面向工资跳槽 ---- 但和 hr 说是因为我就太让人无语了... -xmlhttprequest(build5381); 2021-12-4 (#14157160@0) +1

你们公司也是的，合同工走还做什么HR interview, 我们这里都是直接走人。不过你就来一句他水平差就行了，不需要解释那么多。 -househot(花开花落); 2021-12-4 (#14157177@0) +1

有些合同工很客气，说从我的 code review feedback 里面学了很多（简化 code 的技巧）。但我是一朝被蛇咬十年怕草绳，之后 code review comment 写得是客气客气再客气，委婉委婉再委婉... -xmlhttprequest(build5381); 2021-12-4 (#14157198@0) +1

本该如此 -moonhalf(减肥减多久); 2021-12-4 (#14157454@0) +2

本来的态度是 professional，之后变成表面上更 political correctness -- 但其实，你懂的... -xmlhttprequest(build5381); 2021-12-5 (#14160678@0)

是啊。就是这样，以不得罪人为主。如果不是自己开的公司的事，何必呢 -moonhalf(减肥减多久); 2021-12-5 (#14160976@0) +1

哦，你误会了 --- feedback 更客气并不代表就 code 就随便让过。让一个三个月的合同工 check-in 一堆烂 code 拿了钱走人，然后 team 里永久工兄弟替他擦 pg? 那种对不起 team 兄弟的事儿不能做... -xmlhttprequest(build5381); 2021-12-5 (#14161018@0)

那是烂code不能进，说话嗨不能得罪人，其实码农也是艺术活 -moonhalf(减肥减多久); 2021-12-5 (#14161142@0)

不能说水平差，这么说就是直接羞辱了招这个合同工的人 -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-4 (#14157311@0)

我不知道自己什么时候停止解释了。我自己觉得没有错，如果没有 request 要求我必须提供 evidence，我一般就略过，很快就忘记了。 -troyd(小C); 2021-12-4 (#14157155@0) +1

你是老板耶 ---- 彪悍的人生当然无须解释 lol -xmlhttprequest(build5381); 2021-12-4 (#14157167@0)

合同工这样中途退去合同有些不professional。要找到好的合同工不容易。我们这里今年已经辞退n多个合同工。实在是技能太不行了。 -hutuhao(平庸); 2021-12-4 (#14157219@0)

拿出亚马逊的算法题考考，不就证明技能行了? -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-4 (#14157306@0) +1

招来做front-end的要什么高超算法？算法考的好但连HTTP的GET和POST都分不清的不适合做拿那高薪合同工。 -hutuhao(平庸); 2021-12-4 (#14158172@0) +1

那四大玩命考算法做啥，吃饱了撑的? 难道大家都想证明黎曼猜想? -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-4 (#14158542@0) +1

嘿嘿你较真了。做网络系统前台编程的真不需要那么多知识。 -hutuhao(平庸); 2021-12-5 (#14161204@0) +1

其实说个理由就行，根本不需要挖那么多证据，不需要解释那么细。hr就是走过场，其实那个人这么快走了，就说明问题。而且你还在干活，孰轻孰重老板和hr都知道。 -dandandoudou(doudan); 2021-12-4 (#14157445@0)
是个得罪人得差事。 -ff2021(ff2021); 2021-12-4 (#14157451@0) +3
这种review对于一个senior professonal 就是一种侮辱，管理层想出来的奇葩方案，不知所谓。结果要么不了了之，要么是一走了之。 -txn(秦川); 2021-12-4 (#14158201@0) +4

从他提交的 code 质量来说，证明了 code review 是完全必要的。 -xmlhttprequest(build5381); 2021-12-4 (#14158389@0) +4

那他反过来review你呢？是不是也这样可以说出一大套同样的话，仅仅就编程风格格式不顺眼，究竟对performance什么影响？ -txn(秦川); 2021-12-4 {316} (#14158430@0) +1
这种影响在现在运算力微不足道吧。当然，如果他的水平不符合他现在的职位，完不成任务，直接让他走人。说明招人有问题，做一点事情，还要被人检查作业的哈，真是闲的，幼儿园大班呀？我其实也不是针对你这个具体的案例，只是针对这种方式。这就是针对那种印度烂人的scrum, micromanaging。这就是印度人一统江湖，这种方式才开始变得有意义。

我不介意任何人 review 我的 code 并会感谢好的 feedback。第一个 PR 就一半冗余 code，这不是一个 Sr professional 该有的水准。 -xmlhttprequest(build5381); 2021-12-4 (#14158445@0) +2

站这 -newrover(漫游); 2021-12-4 (#14159203@0) +1

这个人犯的错误真的比较低级。但凡他有点重构的概念都不会犯这种错误。这不是编程风格各异的问题，是代码质量堪忧。 -newrover(漫游); 2021-12-4 (#14159202@0) +1

人就不想费那功夫。 -infinity7.1(notailor); 2021-12-5 (#14159403@0) +2

关键这错误太低级了。没有金刚钻，别揽瓷器活呀😂 -newrover(漫游); 2021-12-5 (#14159590@0)

不揽吃什么? -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-5 (#14159620@0)

他如果只是技术差，可能还能混；这种人品，真该喝西北风😂 -newrover(漫游); 2021-12-5 (#14159738@0)

见下 -keysi(K.S); 2021-12-4 (#14158410@0)
can't agree more. Code review can be used as a bad politics to drive out good nuts. "If I do not pull you down, you will threat my job security. Thus I have to use code review to let everyone know your code sucks". -huangexiaoyu(暑往寒来春复秋); 2021-12-4 (#14158415@0) +2
也不能这么绝对，线上系统怎么也得保持个99%的可用，要不用户流失，BUSINESS抱怨也够头大的，这玩意得从工程角度考虑，别从管理角度考虑。 -ff2021(ff2021); 2021-12-5 (#14161073@0)

现在干IT三年以上的就是Sr., 水准参差不齐，code review很有必要，也是IT Due Diligence的要求，code不光是能运行就行了，更重要的是确认source code 完整，并符合规范，保证没有后门 -keysi(K.S); 2021-12-4 (#14158406@0) +1

所以让三年的假sr来review你的code？ -less_is_more(二手老西医); 2021-12-4 (#14158496@0) +1

code review就是造成developer之间矛盾罪魁祸首。 -less_is_more(二手老西医); 2021-12-4 (#14158498@0) +5

挑动群众斗群众 -big-foot-is-a-joke(big-foot-is-a-joke); 2021-12-4 (#14158539@0) +1
造成developers之间拉帮结派。不是我这一派的不让通过，让你反复地改到想辞职。是我这一派的垃圾code 也能过。 -huangexiaoyu(暑往寒来春复秋); 2021-12-4 (#14158942@0) +1

那是公司文化不行，跟code review 无关。我倒真希望一个高手review 我的code。只要他真能挑出毛病，相当于给我培训了 -newrover(漫游); 2021-12-4 (#14159201@0) +2

是啊。code review 是"权力"，但也是“责任”，一般都是 team lead 来做，一是帮助实施规范，也是帮助组员提高的方法之一。不知道怎么在楼上的公司成拉帮结派的工具了 lol... -xmlhttprequest(build5381); 2021-12-4 (#14159238@0) +1

我们一直讨论的是peer review，专人review不是一回事。 -less_is_more(二手老西医); 2021-12-4 (#14159243@0)

没有team lead 怎么拉帮结派？ -sxffff(lookingforjob); 2021-12-4 (#14159319@0)

赞同,握手🤝。有人的地方就有江湖，code review 变成打击异己的工具也不奇怪。但这不是code review 本身的问题。个人认为code review对于提高代码质量很重要 -newrover(漫游); 2021-12-5 (#14159589@0) +1

我这里的合同工写code也是这种问题，我都不敢提意见，一说点什么，另一个啥也不懂的家伙就赶快出来护着他。我只能干脆和他划清界线，不做同一个项目。 -mapleforum(maple_wizard); 2021-12-5 (#14161421@0)

team lead哪里有那么多的时间review每个team member的code，end of sprint, -sxffff(lookingforjob); 2021-12-5 {298} (#14160127@0)
大家都checkin几千行code，如果执行统一标准，每个team member都有要改的地方，肯定被项目经理骂，随便看看，不引起down机的话就好了。经过好几个项目，都是走过场，大家都开心。一段时间后，发现不行了，找外部的专家再来review一遍，该改再改，如果走到这一步，谁都面子不好看，就推项目时间紧，code质量不好也没办法。

一般就是新员工前几个 commit 仔细 review 一下，立个规矩，也让新员工熟悉一下 team 的标准规范。之后就是自己按照规范来就好了。要是说每次 check in 都不符合规范，要么该被 fire 掉，要么这个公司水平就这样，大家一起 happy 好了。 -xmlhttprequest(build5381); 2021-12-5 (#14160624@0)

你们居然不是每个commit都review？好多项目都要最少一个approval 才能merge。 -sxffff(lookingforjob); 2021-12-5 (#14160840@0)

每个 commit 都要 review，但多严格多仔细就看是哪家公司哪个项目了... 象我说的，新人头几个严一些，核心模块 / security 之类的当然也得仔细... 其他的就看每个 team lead 的项目焦头烂额程度了... -xmlhttprequest(build5381); 2021-12-5 (#14160894@0)

@Ottawa

Topic

More Topics