好好写代码

举报
G-washington 发表于 2020/02/14 21:04:04 2020/02/14
【摘要】 开发者提交了PR其实就是潜意识的已经认为自己的代码写的还不错,完成了工作。评审者提意见的表达方式就很重要,弄不好就是矛盾。这可以让很多人本身并不喜欢或者被动的做评审和被评审。

之前有同学想了解豆瓣的工程管理、CodeReview,代码规范。我先铺垫一下 @段念 和 @清风 2位老师的分享:

1.前豆瓣工程副总裁段念谈豆瓣的研发管理

2.文化建设是个“系统工程”

3.工程师文化中的工具“情结”

4.豆瓣 CODE 两年历程回顾:git 不是万能的,没有 review 是万万不能的

5.清风在 C2D2 的分享(Slide)

说句场外话,就是因为看了清风老师在C2D2的分享,我才决定来豆瓣的。

在这里聊一些我在豆瓣的一些实践和个人理解。

团队要有自己的规范

开发者提交了PR其实就是潜意识的已经认为自己的代码写的还不错,完成了工作。评审者提意见的表达方式就很重要,弄不好就是矛盾。这可以让很多人本身并不喜欢或者被动的做评审和被评审。

实现同一功能,开发者可以选择多种方式,这就是编程的乐趣。但是问题是没有那么多事情是如「太阳东升西落」这样的客观事实,大部分是很主观的,我说这样实现没有问题,你觉得那样可能会更好,谁也说服不了谁。

「不以规矩,不能成方圆」。 团队要有自己的一些规范和要求,大家习惯了就会遵守,也就不会有那么多的不良情绪。而且这些是根据经验不断改进产生的,也会帮助新人少走弯路。

我们对撰写Pull Request、提供反馈、针对反馈的回应、合并代码等方面都做了比较密集的建议,这个和一些开源项目对贡献代码有一些具体要求是一样的。最近我还引入了Github的PR模板ISSUE模板

提PR的时候会要求你针对情况填写:

image.png

提ISSUE的时候会要求你针对情况填写:

image.png

有人可能会说,闲的吧? 你确定你那坨代码4个月后你还能讲清楚它是干什么的? 你确定新人维护你的代码时候不会骂你?

事实上,我个人对commit message也觉得应该加一些要求,但是我们目前没有做到。我有时候看着版本库的提交历史很心痛。

除此之外,我们对代码规范也是有要求的,比如:

1. python 代码(包含 mako 模板的 python 代码部分)使用引号时请使用「单引号」

2. Mako 模板变量左右都需要有空格

3.python代码的docstring首行不直接换行

4.相对引用(relative import) 和绝对引用(absolute import)的具体操作规范

5.SQL编写规范

我们甚至基于业务有一系列的SEPs(Subject Enhancement Proposals)。

有了规范还要坚持。之前发生过一次非常大的对于规范要求的讨论,一个其他组的成员忽视代码规范要求,来提PR被拒绝,继而多个并不在意代码规范的同学来参与。在这里感谢 @VeryCB,通过我们的坚持,之后再也没有发生过不遵守我们团队规范要求的事情。

个人习惯需要向「团队习惯」或者「项目维护者」的习惯做出妥协,在给开源项目贡献过代码的人都应该知道。在团队达成一致的过程中,针对细节的讨论是不可避免的,但是应该鼓励这种讨论。我一直和别人说,在团队按照大家都接受的方式,但是并不要求你认可,如果自己写项目你还是应该遵从内心,其实我有好多时候并不觉得别人的方案是最好的,但是还是认可的,这就够了。如果大家都是一个模子,程序员这个职业也太无聊了。最后再强调,注意方式方法。

有些PEP8没有做要求的规范,也曾经进行激烈讨论。比如「带冒号的二元表达式左右空格的定义」。就是有一天,我想要改进这个PEP8中的空白和其他的1个内容,发了个PR。然后我们组是这样讨论的:

image.png

结束。 你以为故事就这样结束了么?

有一天,一个不归我们组维护的项目的一个PR莫名的@了我,修改中有如下一段句:

image.png

另外一个同学质疑这是否符合pep8,我又顺便安利了一下:

安利成功!

一般遇到有争议的点,我一般有如下惯例:

1.看Python标准库中有没有对应的证据

2.看优秀的、流行的第三方库中有没有对应的证据

3. 看厂内有没有对应的先例

使用合适的工具

有句话叫「工欲善其事,必先利其器」,我们来看看工作中我们用到了那些工具。

1. pre-commit/pre-commit。 持续集成是现在互联网公司必备的一个环节,豆瓣一个专门用来跑CI的服务器集群,你提交PR的时候就会触发,代码能不能被合并都靠它的结果。如果你提交了一个本来就不符合代码规范之类的PR除了可能被吐槽外,还占用了持续集成服务器的资源,尤其是项目比较大的时候,跑一次就要10几分钟,那么这段时间是浪费的。Git支持多种hook(钩子),在你commit或者push代码的时候就可以触发本地的检查,这样本地就可以预先检查出一些问题了。BTW, pre-push功能是我加的。

2. dongweiming/gandalf。在做代码评审时,无论你多么注意方式方法都还是会让对方不咋愉快的。我把代码质量检查作为持续集成的一部分,这样CI挂了,你就看到了。而且我们现在感觉有点默契的潜规则:如果你的CI都没有过,一般都是没人理的,直到你修改的让CI通过。我觉得这些场景人总是像啄木鸟一样出来不断的「啄啄啄」,累还招人烦在,怎么办呢?让程序自动的来发这些检查意见,看下效果:



这样就不用人工的去订正了,解放了生产力。

3. facebook/mention-bot。现在这个工具已经被我推广到整个豆瓣。有时候一个人发了一个PR,他并不知道该提及(@)谁(我一般是看最近活跃的开发者)。有时候其实会@错,别人又会@其他人... 等到真的维护者来时间已经过去很久了。这个工具就是通过算法分辨你在这次的修改谁会更有兴趣继而决定提及谁。用了一年时间,感觉还是蛮准的。缺点是有时候有点烦,因为可能在某个时间上你已经不维护它了,但是由于最近的一些修改是你而被提及。

4. AST。Pylint、PyChecker和PyFlakes都是基于它做的静态检查。但事实上代码规范检查只是一部分内容,其实任何可度量的业务上的东西也都可以检查。我举几个例子:代码中对应的位置有没有加缓存?缓存有没有被正确删掉?用法有没有遵守惯例?SQL拼的有没有安全问题等等。


作者:董伟明

链接:https://zhuanlan.zhihu.com/p/22338225

来源:知乎


本文转载自异步社区 

原文链接:https://www.epubit.com/articleDetails?id=NC7E3EF917D50000169291C5EE1F014D0

【版权声明】本文为华为云社区用户转载文章,如果您发现本社区中有涉嫌抄袭的内容,欢迎发送邮件进行举报,并提供相关证据,一经查实,本社区将立刻删除涉嫌侵权内容,举报邮箱: cloudbbs@huaweicloud.com
  • 点赞
  • 收藏
  • 关注作者

评论(0

0/1000
抱歉,系统识别当前为高风险访问,暂不支持该操作

全部回复

上滑加载中

设置昵称

在此一键设置昵称,即可参与社区互动!

*长度不超过10个汉字或20个英文字符,设置后3个月内不可修改。

*长度不超过10个汉字或20个英文字符,设置后3个月内不可修改。