遇上一个对Code Quality 超级执着的组员，快被逼疯了，经验

donnice 发表于 2023-3-18 23:29
如果之前都是屎，老组员决心肃清屎山，然后一个改动挺大的，涉及到一部分历史屎山，那应该老组员自己修。 ...

看来我也是脾气太好了，我碰到写的东西中间有别人写的屎山，不是特别麻烦顺手也就修了。。。我换组了也懒得去看那个cl 提到的历史屎山是不是修起来特别麻烦了。

匿名用户 发表于 2023-3-19 14:33
看来我也是脾气太好了，我碰到写的东西中间有别人写的屎山，不是特别麻烦顺手也就修了。。。

我也是这样的人呀，如果不麻烦的小改动，改改local变量名什么的，改了就改了，太脏的代码我也不能忍。但如果是比较大的屎山，就算不考虑delivery，我也会单独开一个TODO item去修，而不是顺手修掉，因为太麻烦。楼主这种情况，至少我读下来显然没那么简单，更接近于后者。如果真的是所谓的“小”改动而楼主上来发贴抱怨，那我也站在你这一边

你这关于code review的吐槽已经被review飞起来了…而且已经开始趋向越来越nit picking，大家都开始较真。程序员做久了容易有这个职业病，我特别能理解，有的时候要学会适度lay back，任何事情都是

donnice 发表于 2023-3-18 23:37
我也是这样的人呀，如果不麻烦的小改动，改改local变量名什么的，改了就改了，太脏的代码我也不能忍。但 ...

我最近遇到一个类似的case不过我是写comments的那个。我不知道楼主吐槽的人是不是我。

我昨晚看到帖子之后，半夜爬起来又去看了看我的comments，我也好好反思了一下。确实其中有不少都是nit on code quality，没料到会给同事造成压力确实是我不好。对这位同事以后的代码，我会少写一些comments，多当面沟通，尽快approve。

当然我这边也是有自己的压力的。当tl之前，我review新人代码经常放水，已经被mgr批评过多次。

希望我们互相体谅。

下面是对d老兄的质疑的一些小回复

1. 为啥我要在在屎山上强调code quality？code quality是mgr最近才开始强调的。原因是我们组产品已经逐渐稳定，从fast dev堆屎山变成slow evolve搭危楼，尤其是之前几次outages，所以mgr明确和我提出，希望我进一步提高代码审核的bar，features可以慢一些，但新代码的bar一定要hold住。

2. 我干嘛要写那么多nit comments？因为无论对这位同事还是对我来说，由我写出来都好过由mgr写出来。
3. 为啥我不帮忙refactor屎山？因为我也刚刚接手还不到一个quarter。我之前lvl比这位新同事还低一级，去年年底才promo到这位新同事同级。我之前是主要负责另一个版块的，那边我一直很注重代码质量。同事现在work on的这座屎山之前不是我的项目。我现在接手之后只能做到不堆新屎。

匿名用户 发表于 2023-03-19 00:33:12
我最近遇到一个类似的case不过我是写comments的那个。我不知道楼主吐槽的人是不是我。

我昨晚看到帖子之后，半夜爬起来又去看了看我的comments，我也好好反思了一下。确实其中有不少都是

天呐，你脾气真好。真是看不下去了，我即使看楼主说的那三点，我也没觉得那个senior做的有什么不妥的地方，我同意楼里所有支持code quality的人评论。说真的，耐心反馈算不错了。真的像这个论坛里面那些给一组印度人的干的，你爱堆什么山堆什么山，反正给你的活都是没人爱干的，你就知道是什么心情了。

匿名用户 发表于 2023-03-19 00:33:12
我最近遇到一个类似的case不过我是写comments的那个。我不知道楼主吐槽的人是不是我。

我昨晚看到帖子之后，半夜爬起来又去看了看我的comments，我也好好反思了一下。确实其中有不少都是

可以理解，上面针对你的言论是我不了解情况，只是根据楼主的一面之词以及之前的惨痛经历做出的不当判断，在此给你道个歉。其实如果你有机会直接和楼主同事聊清楚这些情况的话，那我认为你的同事是完全可以理解你的。

言归正传，我个人认为比起来回在一个新feature的PR里纠结，更好的一个方法是开一个组会，告知大家现在组里对code quality的要求提高，今后写代码要注意12345678。然后针对现有的屎山，花几个sprint，在做新feature的同时，开几个ticket来做针对性的屎山refactoring，每人都要平均分配到一些，能在兼顾效率的同时把代码质量提上来。这样做有几个好处，一个是明确义务，因为code quality在传统意义上不太作为一个人review的根据（因为很难定性，除非是写得太差没法看的那种），所以很少有人能非常重视，现在就明确进入KPI了，所有人自然都会当回事，心理上也会不再抵触。第二个也是更重要的，告知mgr，既然你要求code quality，我们现在要开始为了这个要求invest time了，那feature开发肯定会受影响，让mgr也有个心理准备。第三个是single responsibility，也就是一个PR就为了一个任务，不要在开发新feature的时候还要兼顾大型的refactor，让开发者和reviewer都有个明确的任务指向。第四个是明确标准，因为code quality的标准其实并不统一，通过这个组会可以明确对code quality的要求究竟是什么，这样大家都有个方向，不至于沦为PR里的口水战。最后一个也是对层主你有好处，show ownership，以后组里代码质量高自然主要功劳都在你头上，我也认为你配得上这份功劳。当然，层主有心的话，还可以主动引入一些自动化代码质量检查的工具，让大家在提交PR前就能过一个baseline。不知层主怎么看？

donnice 发表于 2023-3-19 02:38
可以理解，上面针对你的言论是我不了解情况，只是根据楼主的一面之词以及之前的惨痛经历做出的不当判断， ...

你说的很有道理，基本完全同意。我们事实上也大致是这么做的。

第一点具体化kpi我们有做，product excellence是我们今年的okr之一。

第二点和mgr沟通，这点上我们应该是in sync的，而且feature慢一点也要把关质量是ta提出来的。

第三点一个pr一个目的，我是非常支持的。我自己也是喜欢split小pr。甚至一个任务都会想要不要用两三个pr来。这次我要求的refactor是基于新加入的代码的，比如新定义的method是不是可以优化输入输出的结构，新代码里面那种3/4层的nested if是不是可以用early break来降低层级，新method虽然很简单，但是不是可以写个unit test cover一下。当然最后一个有点争议，因为copy的旧method是屎山缺少现成的unit test，从零开始搭unit test确实麻烦，往往写个test比写method本身还费劲。

第四点明确要求，这点其实我司内部有代码规范。一般写code style的comments我都会尽量找一下规范原文链接或者截图附在comments上，方便pr作者理解。

补充内容 (2023-03-20 02:20 +8:00):
我又看了一遍对话，现在我深刻觉得，给自己的行为找理由美化开脱是人之常情。我的解释只是给我自己甩锅，并不是在尝试解决问题。

我觉得实际上到具体事务上主要还是靠沟通。楼主的case以及我的case，其实内核不一定是pr review的问题，而是同事之间沟通协作的问题。（不得不说这个帖子的sub topic选得真对）

沟通问题尤其是新人进组最难，因为渠道过于单一。如果是remote就更是雪上加霜。主要是两个方面
1. 新同事不好意思说话。“这些comments有点block我的进度，你看看哪些我可以挪到以后再改？”老同事之间这么说很轻松，但是新同事就很难当面直接说出这种话。
2. 新同事对老同事的意见会非常重视。“这次comments有点多，但都是小comments，整体都ok。”老同事会按照字面意思理解，但新同事会担心是不是自己的成绩得不到认可。

这些我也没啥好的解决方法，我目前工作经验很短，好多东西也在慢慢探（踩）索（坑）。

总之慢慢磨合嘛。

Call out scope and non-scope of your project in your design or project plan doc, and get manager's signoff.

In the non-scope section, call out the code quality of existing code. Once it is signed off, if reviewer asks for changing the existing code, just refer it to the doc that it is not in the scope of the current project, and it should be in a separate effort. If he insists, escalate it to the manage.

if they would like to improve the code quality of the existing implementation in your project, reestimate the timeline to deliver the final product and get it signed off as well.

有道理的，谢过以后改
没道理的，怼回去
然后记住每次提交CR以后，战斗才算真正开始

很多时候几个方案都可以也都各有利弊，要学会如何在尊重他人观点的情况下说服别人同意你自己的解决方案，这个技能很重要，在CR里面撕逼扯淡是很好的练习方式。

楼主至少认可人大哥的建议是正确的，以前遇到一新来的傻13天天堆屎，还觉得自己写得很好，别人的意见听不进去。