說到code review這件事,相信這年頭凡是吃碼農這行飯的,多多少少已經成為工作的一部分了( 啥?貴公司沒在做這件事甚至反對這件事?塊陶啊~~~)
如果估狗『how to code review』、『code review guidelines』等關鍵字,很容易就可以找到許多不錯的參考文章,但很多都是超過十項的點檢表,我實在不擅長記憶這麼多項目,常常邊看邊忘,在腦內只留下了浪漫模糊的輪廓。
點檢表是非常有用的,特別是在初期對一個code base還很不熟悉的時候。例如咱家WordPress VIP code review guidelines、WordPress theme development checklist、這篇Kevin London的Code Review Best Practices也曾被轉載一陣。待對code base熟悉到一個程度以上後,記憶幾個好記的思考源作起點再從之延伸,對我來說比較容易掌握:
- 我了解整個patch / PR的目的嗎?
- 我知道該怎麼測試嗎?
- 每一行變更的意義我都完全了解嗎?
- 我們能做得更好嗎?
從『目的』延伸
從『了解目的』的延伸較單純,主要就是要求用意清楚、單一,這通常代表了這個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表面上讓進展變慢、長期上是變快,我覺得倒不如說這才是正常的步調,省去之後的快本來就是假象。
對「關於Code review這檔事」的一則回應