保證代碼質量二三事

之前有同學想了解豆瓣的工程管理、CodeReview，代碼規範。我先鋪墊一下 @段念 和 @清風 2位老師的分享：

1. 前豆瓣工程副總裁段念談豆瓣的研發管理

2. 文化建設是個「系統工程」

3. 工程師文化中的工具「情結」

4. 豆瓣 CODE 兩年歷程回顧：git 不是萬能的，沒有 review 是萬萬不能的

5. 清風在 C2D2 的分享（Slide）

說句場外話，就是因為看了清風老師在C2D2的分享，我才決定來豆瓣的。

在這裡聊一些我在豆瓣的一些實踐和個人理解。

團隊要有自己的規範

開發者提交了PR其實就是潛意識的已經認為自己的代碼寫的還不錯，完成了工作。評審者提意見的表達方式就很重要，弄不好就是矛盾。這可以讓很多人本身並不喜歡或者被動的做評審和被評審。

實現同一功能，開發者可以選擇多種方式，這就是編程的樂趣。但是問題是沒有那麼多事情是如「太陽東升西落」這樣的客觀事實，大部分是很主觀的，我說這樣實現沒有問題，你覺得那樣可能會更好，誰也說服不了誰。

「不以規矩，不能成方圓」。 團隊要有自己的一些規範和要求，大家習慣了就會遵守，也就不會有那麼多的不良情緒。而且這些是根據經驗不斷改進產生的，也會幫助新人少走彎路。

我們對撰寫Pull Request、提供反饋、針對反饋的回應、合併代碼等方面都做了比較密集的建議，這個和一些開源項目對貢獻代碼有一些具體要求是一樣的。最近我還引入了Github的PR模板和ISSUE模板。

提PR的時候會要求你針對情況填寫：

What does this PR do?nnWhich issues does this PR fix or reference?nnTrello or Product requirement document pathnnHow can we reproduce the issue?n

提ISSUE的時候會要求你針對情況填寫：

Whats the problem (or question)?nnWhat should have happened?nnDo you have an idea for a solution?nnHow can we reproduce the issue?n

有人可能會說，閑的吧？ 你確定你那坨代碼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。然後我們組是這樣討論的：

A說: nnhttps://www.python.org/dev/peps/pep-0008/#pet-peevesnn其實 PEP8 對這裡沒有做非常死的限制，事實上nham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]nham[lower:upper], ham[lower:upper:], ham[lower::step]nham[lower+offset : upper+offset]nham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)]nham[lower + offset : upper + offset]nn這些寫法都是 PEP8 推薦的nnB說：nnpep8 +1nn按照上面的例子 list[start : start+limit] 或者 list[start : start + limit] 都是比 list[start:start + limit] 要好點nn我說：nn閑來無事 剛才翻了了我本地的一些項目(先排除不care代碼規範的項目). 還看了 sentry, httpie, flake8, twine, warehouse. werkzeug, flask_admin等除了沒有這樣的用法機會. 就沒有其他表達這樣用法的方式. nn我檢索的代碼的方式是 grep ":" ./ -rn|grep "["|grep "]"|grep "+" nnpip也是贊同我的 —— PS: pypa這個組織的人大多是python核心開發者, 以及社區重要業務維護者nnhttps://github.com/pypa/pip/blob/develop/tasks/generate.py#L99nn但是我看到一個更特別點的用法:nnhttps://github.com/getsentry/sentry/blob/master/src/sentry/templatetags/sentry_helpers.py#L70nn[i:(i + break_after)]nnC說：nnpep8 里說把這裡的冒號當作一個二元操作符，兩邊的算其參數：nlist[start:start + limit]nn這樣的寫法看上去要表達的是 start 和 start 的關係更緊密， : 優先順序更高，但是實際上 + 的優先順序更高，這也是你列出的最後一種寫法為什麼要加上括弧的原因，這樣確實清晰很多，但是每次都打括弧有些冗長。為什麼不用我提出的那種：nlist[start : start+limit]nn這樣更為清晰的表達了操作符的優先順序和各個參數之間的親密關係，並且也是在 pep8 給的 examples 里的：nham[lower+offset : upper+offset]nn我說：nnham[lower+offset : upper+offset] 但是嚴格意義不是 ham[lower: upper+offset] 這個的格式.nn其次我看 list[start:start + limit] 很舒暢啊 , list[start : start+limit] 略怪. 略怪的原因在於運算符直接沒有加空格.nn @subject 投票時間到了. nn* 我傾向尊重python標準庫以及開源項目的風格, 堅持 list[start:start + limit] n* @C list[start : start+limit] n編程風格也不一定一定就是統一的一個風格, 要不然這個地方就不做要求, 大家按自己的習慣?nnA說：nn看完大家的討論我傾向於這個地方不做規範，按個人風格來寫+1nnB說：nn:lgtm:n

結束。 你以為故事就這樣結束了么？

有一天，一個不歸我們組維護的項目的一個PR莫名的@了我，修改中有如下一段句：

- group_chats = group_chats[start:start+count]n+ group_chats = group_chats[start:start + count]n

另外一個同學質疑這是否符合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拼的有沒有安全問題等等。

無恥的廣告：《Python Web開發實戰》上市了！

歡迎關注本人的微信公眾號獲取更多Python相關的內容（也可以直接搜索「Python之美」）：

推薦閱讀：