代碼審查的藝術:Dropbox 的故事,審查dropbox
Dropbox 的 iOS 應用中的每一行代碼,都是開始於被添加到 Maniphest 中的一個 bug 或者功能任務,Maniphest 是我們的任務管理系統。當一位工程師在上面接受一個任務,那麼在開始寫代碼前相應的責任就已經賦予他。Phabricator 這個平台包含了我們的代碼審查工具,這個代碼審查工具有很多很好的功能,但它在評估對象之間的相互協作上不是做的很好。為了彌補這點,我們的工程師在開始他們的工作之前需要知道審查他們的任務的人是誰[1]。對於被審查代碼的工程師來說,這樣能確保在他們的團隊中有一個橡皮鴨,這個橡皮鴨知道項目中一些改動代碼的背景和原因,並且對代碼的設計決策上起到協助的作用。對於審查者來說,這有助於他們將一些變化考慮進他們的開發週期評估中,這樣有助於開發週期評估的準確。如果不出意外的話,我們的經驗會告訴我們提前做好計劃可以有效地避免審查代碼過程中的重複勞動。針對項目中的變化做計劃可以像在白板前做交流一樣簡單,也可以像寫一篇建設性文檔一樣深入。這都取決於我們自己的選擇。
[1] 我的團隊中每個人都要審查代碼。新來的同事在可以獨立審查較大的任務之前,會先被分配一些比較少的代碼量。
隨著任務的展開,工程師需要一直謹記我們的代碼規範。這個規範是一個最佳實務和一致規範的大融合,它的存在使我們不用去猜測我們應該怎樣編碼,也使審查變得更容易[2]。因為這是一個大項目,Team Dev中沒有一個人能對整個項目有完美的映射或理解。所以我們的工程師需要依賴團隊中其他工程師的協助,將這些代碼的功能表現拼成一個整體,這有助與我們在閱讀代碼時能理解其中的邏輯。
[2] 即使這樣,每當一個新成員加入時,總還是不免要展開一次關於使用 property 還是 ivar 的辯論。
當這個任務的工作進行到某個階段時,我們的工程師很可能會做出一些明顯不合理的或者不受歡迎的決定。捕獲這個心理的最佳時間就是發生這一刻 — 為將來向審查者做好解釋的準備。去解釋這些變化,說起來容易做起來難,我們的工程師被鼓勵使用//TODO
,//HAX
,和 //FIXME
來在代碼中寫注釋。//TODO
和 //FIXME
從字面上就可以理解它的意義,儘管後者會產生編譯警告,所以必須在下一次發布之前要被解決。//HAX
這個注釋比較有趣的地方。我們用它標註那些用來繞過 Apple 的 API 裡的 bug 但又不容易一眼看明白的方法[3]。我們的注釋會寫上日期和寫這個注釋人的名字[4],在之後很多時候我們總會感激這些額外的內容相關的[5]。
[3] 標註裡通常是第三方來源或者 radar 的連結,還有特殊的重現步驟。
[4] 比如像 //HAX:(ashleynh) 2015-03-09