關於Code review這檔事

說到code review這件事,相信這年頭凡是吃碼農這行飯的,多多少少已經成為工作的一部分了( 啥?貴公司沒在做這件事甚至反對這件事?塊陶啊~~~)

如果估狗『how to code review』、『code review guidelines』等關鍵字,很容易就可以找到許多不錯的參考文章,但很多都是超過十項的點檢表,我實在不擅長記憶這麼多項目,常常邊看邊忘,在腦內只留下了浪漫模糊的輪廓。

點檢表是非常有用的,特別是在初期對一個code base還很不熟悉的時候。例如咱家WordPress VIP code review guidelinesWordPress theme development checklist、這篇Kevin London的Code Review Best Practices也曾被轉載一陣。待對code base熟悉到一個程度以上後,記憶幾個好記的思考源作起點再從之延伸,對我來說比較容易掌握:

  1. 我了解整個patch / PR的目的嗎?
  2. 我知道該怎麼測試嗎?
  3. 每一行變更的意義我都完全了解嗎?
  4. 我們能做得更好嗎?


code-review-pillars

從『目的』延伸

從『了解目的』的延伸較單純,主要就是要求用意清楚、單一,這通常代表了這個patch / PR不能太大、盡可能只有單一個目的、說明必須清楚。不論對reviewer或作者來說,這些都是在進入程式碼前就該做到的事情。

從『測試』延伸

有unit test就要跑、有相關integration test要跑,如果是跟使用者情境有關的,一定要去實際用用看;用潮一點的話來說,就是要做e2e test。作者應當要把整個測試計畫寫清楚,力求reviewer可以挖著鼻孔傻傻跟著步驟做就可以完全測試完。此一基本要求做到後,才是reviewer把手指擦乾淨,該發揮點創意的時候了:想辦法把測試弄壞

對unit test或automated integration test來說比較簡單,挑其中幾項把input弄成『理應出錯』的狀態,新加的測試尤其要試,確認這些測試不是在那邊擺好看的。這件事在php或js這類語言上尤其重要,因為語言本身能給的正確性保證實在是少得可憐。舉個例,下面是我最近碰到的一個case的簡化版:

describe( 'just a example', () => {
    it( 'func a should have been called.', () => {
        const func = sinon.spy();
        somethingThatCallsFuncOnEven( 2, func );
        expect( func ).to.have.beenCalled();
    } );
} );

假設somethingThatCallsFuncOnEven是個當第一個參數是偶數時會call func的函式,上面這個測試理當改傳入1, 3, 5等奇數就會報錯,但它還是過了,為什麼?因為beenCalled永遠都是true,正確的呼叫法是been.called()。我沒去研究為什麼會有這麼奇怪的行為,但這是我真切碰到的案例。

使用測試嘛…好像就沒什麼系統化的方法,我個人的方法是『想像我沒看過程式碼』。我以前在鈊X工作的時候有幸認識幾位這方面非常有天份的測試員,他們總是能做出我想都沒想過的動作把程式搞爆;這邊連按幾下、那幾個鍵一起按按看、這邊好像可點耶~點點看…啊,爆炸了 ^.< 諸如此類。從他們身上,我發現我作為『知道實作』的人,太過清楚『預期中的使用流程』了,很容易有盲點。 當然我不可能完全沒看到程式碼,不然怎麼做code review? 但在做這部分測試時,適時把視點切換、改從使用情境著眼,我的經驗上非常有幫助。

了解每一行的意義

特地強調『每一行』是為了從一開始就有精度上的心理準備。我覺得code review非常忌諱『大致上看起來對』,尤其當你的同事們都是高水平的碼農時,如果不拿放大鏡看,很難找到隱藏的瑕疵。搞不清楚的時候,想想自己可能會怎麼做是不錯的方法,常常這樣一來一往的思考下就通了。當然,最後的一招就是直接抓作者本人來問。

我們可以做得更好嗎?

與其說關鍵字是『更好』,其實是『我們』。強調『我們』,是因為我認為code review是合作學習,而非老師審閱學生。有了這一層認識後,再來談改善就會自然、輕鬆許多。

實作方法符合best practices嗎?程式碼能再精簡嗎?命名直白嗎?效能有進步空間嗎?有適當的文件嗎?精微之處有註解嗎?如果手邊有公司要求的落落長的checklist(例如前面提過的WordPress VIP checklist),這是個讓它重新出場的好時機。

總之,快樂code review!

由於個人不擅長記憶太瑣碎的項目,又不喜歡隨時拿張表來檢查,以上四項不論我在哪個code base工作,都是我用來review的基本起點,目前為止至少在我個人還運作得不錯。魯莽deploy真的是很危險的事情,雖然一開始進展很快很有快感,但積累太多髒污最後一口氣噴發的時候,可是會讓覺得自己好棒棒的那些愉悅都打成笑話一樁的。以前亂上程式碼發生過不少慘事,有人曾形容中國近年經濟的問題就是『步子邁太大,扯到蛋了』,我的狀況差不多就是那樣。與其說嚴謹的code review表面上讓進展變慢、長期上是變快,我覺得倒不如說這才是正常的步調,省去之後的快本來就是假象。

發表迴響

在下方填入你的資料或按右方圖示以社群網站登入:

WordPress.com Logo

您的留言將使用 WordPress.com 帳號。 登出 / 變更 )

Twitter picture

您的留言將使用 Twitter 帳號。 登出 / 變更 )

Facebook照片

您的留言將使用 Facebook 帳號。 登出 / 變更 )

Google+ photo

您的留言將使用 Google+ 帳號。 登出 / 變更 )

連結到 %s