一亩三分地

 找回密码 注册账号

扫描二维码登录本站


Salarytics=Salary Analytics
查询工资数据
系统自动计算每年收入

科技公司如何
用数据分析驱动产品开发
coupon code 250off 立减$250

深入浅出AB Test
从入门到精通
coupon code 250off 立减$250
游戏初创公司招聘工程师、UIUX Designer和游戏策划
坐标湾区
DreamCraft创始团队
招聘游戏开发工程师
把贵司信息放这里
查看: 439|回复: 5
收起左侧

[与老板相处 Manage Up] 关于code review

[复制链接] |试试Instant~ |与老板相处 manage up, 职场达人
我的人缘0

分享帖子到朋友圈
lucius323 | 显示全部楼层 |阅读模式
本楼: 👍   0% (0)
 
 
0% (0)   👎
全局: 👍   100% (6)
 
 
0% (0)    👎

注册一亩三分地论坛,查看更多干货!

您需要 登录 才可以下载或查看,没有帐号?注册账号

x
hi 各位好,
本人今年2月master毕业开始第一份工作,在一个不大不小的公司做sde,mentor中国人。

在check in代码之前要将代码上传,我的mentor会进行review,approve后才可以check in.

但是我mentor经常给我提代码中我改动以外部分的修改建议并让我修改。例如一个file我改动了其中的20行,但是他对其他的地方提出了意见并让我修改,并且改动很大。
. From 1point 3acres bbs
要求我改动的大致都是rewrite代码,用新的function来rewrite.

. From 1point 3acres bbs而且组里其它人的代码新写的代码也会有用legacy function的情况,我mentor对于这种情况并不会提醒他们并让他们修改。甚至他自己有一次checkin也用到了legacy函数。个人感觉不是很爽。

但是我听说review代码应该只对改动的地方提出建议,请问下各位这种情况是不是应该和mentor沟通一下,说明其他部分并不应该由我来改动。请问这种情况该如何沟通呢?

多谢各位大侠了. 1point3acres



上一篇:求大家支招:如何和一个爱吹牛但没有什么工作经验的同事相处
下一篇:New grad 第一份工作,要怎么表现
我的人缘0
stonefish 2019-10-21 22:12:27 | 显示全部楼层
本楼: 👍   100% (1)
 
 
0% (0)   👎
全局: 👍   99% (386)
 
 
0% (3)    👎
哈哈哈,楼上说的很好。说说作为code reviewer的感想,一般我们不建议修改legacy code,除非1.公司大范围内deprecate某些东西,那就是看见了就顺手让改了 2.实在是太难看,心里暗骂之前是哪个通过的这段代码?3.代码有错误,以前没发现或者是其他修改带来了错误。这些一般都是小改动,如果有大的改动,就不open issue,只是留个comment说让他们考虑以后修改。
一般出了1或者2,会碰到两种反应,新手一般就乖乖去改了,或者有些比较强硬的,会说out of scope,以后再改之类的。不是原则问题一般我都放过。说以后再改的,如果的确有改的计划,有注释todo,这都是加分项。虽然我们不是mentor review,我觉得ta写得好不好跟他的performance没一毛钱关系,不过以后再碰到了好说话嘛。
我一般不太管别人的code风格,特别是这事每个组都不一样。最烦别人让我就一个变量也要object destruction之类的。
回复

使用道具 举报

我的人缘0
本楼: 👍   0% (0)
 
 
0% (0)   👎
全局: 👍   0% (0)
 
 
0% (0)    👎
当然得沟通了,你委婉的表达一下自己的看法
回复

使用道具 举报

我的人缘0
csushin1992 2019-10-21 13:03:26 | 显示全部楼层
本楼: 👍   0% (0)
 
 
0% (0)   👎
全局: 👍   92% (106)
 
 
7% (9)    👎
本帖最后由 csushin1992 于 2019-10-21 13:05 编辑

这种有很多理由push back的,举几个例子,1,当初任务scope的时候没有考虑到需要refactor legacy code,组里也没有提出需要refactor的意见,这个时候提新需求会endanger ddl, 2. 如果对方用best practice来要求(或者说用这个工作属于集体贡献以提升组里代码质量等道德绑架来要求的话),你就商量着用单独的Code review来改,理由同样是best practice,单独的commit必须要有自己的范围,如果遇到需要revert,就会牵连很多不必要的改变,3. 如果对方硬是说单独的CR没必要,觉得工作量就是一两行的事情,那就跟他来此半小时的1;1,弄清楚对方想具体怎么改,注意这里要具体到每个method内部怎么写,这样他才能理解工作量,4. 如果前三个理由都不行,和manager沟通。这个国人太可恶,纯属刁难你。

至于你觉得mentor没有要求别人改(i.e. 双标)的问题,首先你绝对不能提这个,大忌,绝对不能跟mentor或者manager说双标。从另一方面想,有可能你以偏概全了,也有可能是mentor想考验一下你。如果你能够提出以上理由push back,这也能表现你考虑周全的一面。

最后,个人感想。首先大部分组里有时候是喜欢欺负新人,让新人去做一些dirty work的,也算是块炼金石。如果能够忍着完成且干得漂亮,impressive, 评分A。如果知道能够合理push back, 就说明你有backbone,评分B。如果忍气吞声,又完成不了,下个pip的就是你。

如果觉得有用,求大米~~

评分

参与人数 5大米 +6 收起 理由
dream1995.cp + 2 给你点个赞!
hardcherry + 1 给你点个赞!
caliving98 + 1 赞一个
lucius323 + 1 赞一个
wyang9311 + 1 很有用的信息!

查看全部评分

回复

使用道具 举报

我的人缘0
本楼: 👍   0% (0)
 
 
0% (0)   👎
全局: 👍   100% (6)
 
 
0% (0)    👎
csushin1992 发表于 2019/10/21 13:03:26
这种有很多理由push back的,举几个例子,1,当初任务scope的时候没有考虑到需要refactor legacy code,组里也没有提出需要refactor的意见,这个时候提新需求会enda...
真的太感谢您的建议了,感觉您这些建议都非常有帮助,非常妥当。我最后是和mentor说在完成当前feature之后,再新开一个diff来修改。我还是希望做到把dirty work最好的哈哈哈。之后不爽的话感觉直接跳槽或者转组比较好。anyway非常感谢!
回复

使用道具 举报

我的人缘0
本楼: 👍   0% (0)
 
 
0% (0)   👎
全局: 👍   100% (6)
 
 
0% (0)    👎
stonefish 发表于 2019/10/21 22:12:27
哈哈哈,楼上说的很好。说说作为code reviewer的感想,一般我们不建议修改legacy code,除非1.公司大范围内deprecate某些东西,那就是看见了就顺手让改了 2.实在是太难看,心...
感谢您的回复,我留了个todo以后再改。不过我是觉得这个组氛围不是特别好,refactor code或者fix legacy code都是新人来做,完全没有任何提高。
回复

使用道具 举报

您需要登录后才可以回帖 登录 | 注册账号

本版积分规则

隐私提醒:
■为防止被骚扰甚至人肉,不要公开留微信等联系方式,请以论坛私信方式发送。
■特定版块可以超级匿名:https://pay.1point3acres.com/tools/thread
■其他版块匿名方法:http://www.1point3acres.com/bbs/thread-405991-1-1.html

手机版||一亩三分地

GMT+8, 2019-11-18 22:37

Powered by Discuz! X3

© 2001-2013 Comsenz Inc. Design By HUXTeam

快速回复 返回顶部 返回列表