程式碼審查
程式碼審查的整體目標是作為我們團隊其他成員的安全網,並幫助他們編寫更好的程式碼,而不是評判他們或他們的程式碼。如有疑問,請假設他們有良好的意圖,並且態度友善。
目標
- 找出錯誤
- 找出非顯而易見的方法後果 - 這個 PR 是否會使未來的程式碼更難以保護或更容易出錯。
- 對於未經討論就編碼的情況,程式碼審查可以作為健全性檢查,以確保採用正確的方法
- 指出 PR 對 Metabase 的未觸及部分有何影響
- 指出使用良好方法或風格的地方。程式碼審查不是找碴大會。除非 PR 完全糟透了,否則應該提出同等數量的優點和缺點。
進行程式碼審查的心態
作為審查人員,您的主要目標是作為安全網,防止不良程式碼被合併。「不良」的定義非常主觀、取決於情境,並且會隨著產品時間和成熟度而變化。
當您發現明顯的錯誤時,請花時間註明您認為它們是錯誤的原因。
如果您看到您不同意某些方法的地方,請說出來。但是,也要花時間了解作者做出特定選擇的原因。您應該假設作者根據他們當時所知道的情況做出了良好的決定。您可能擁有不同的知識組合,並看到不同的結果。深入研究這些。他們可能會看到您沒有看到的東西,反之亦然。
尋找您可以學習的技巧、技術或慣用語。您的隊友都是聰明人。他們很可能掌握您可以學習的技巧。重點是讓他們知道。
接受程式碼審查的心態
審查人員正在為您做一件好事。他們在那裡幫助您盡可能做到最好。最優秀的人都有教練、編輯和導師。您的程式碼審查人員應該以同樣的方式幫助您。在他們更有經驗的情況下,這可以是直接指導。在他們資歷較淺的情況下,他們有一雙全新的眼睛,可能會讓您質疑根深蒂固的假設。
當審查人員不同意您採取的方法時,請尋求了解原因。他們可能知道您不知道的事情或看到您沒有看到的後果。雖然他們可能沒有像您一樣深入思考 PR 的特定主題,但他們也可能正在思考 PR 對您可能沒有注意到的領域的影響。
如果有人在您的 PR 上強烈表示「:-1:」,請特別耐心。深入研究他們認為 PR 有缺陷的原因。以改進 PR 而非捍衛您的方法為意圖來進行對話。您在辯論技巧上不會獲得任何分數,但您會在交付更好的程式碼和更好的產品方面獲得分數,無論靈感或想法來自何處。
流程
- 每個具有重大複雜性的 PR 都需要團隊中至少一位其他工程師(或 @salsakran)表示「:+1:」才能合併
- 將您認為應該審查您的 PR 的人員新增至 PR 的受讓人。審查人員可以在審查完成或決定他們不是合適的審查人員後,自行移除
- 影響其他工程師工作的程式碼應由這些工程師審查
- 「:+1:」是預設的「我對此沒問題」
- 「:+0:」(我編造的)是「我對此不滿意,但其他人說「+1」表示它可以合併
- 「:-1:」是強硬的否決。這應該謹慎用於例行 PR 中,並且僅適用於缺少測試、公然違反樣式指南或破壞程式碼庫另一部分所依賴的假設的情況。
- 如果您在未討論設計或與工作可能受到影響的其他工程師討論影響的情況下切出主要分支,則您應該預期會收到「:-1:」,並且不要執著於重新設計有爭議的部分。
- 任何帶有「:-1:」的 PR 在解決之前都不能合併。
- PR 的擁有者和投「:-1:」票的人員應解決方法上的差異。
- 如果出現僵局,@salsakran 會投下決勝票。僵局應該很少見。
請注意,這些「:+1:」、「:+0:」和「:-1:」應在註解中明確說明,而不是在 github 上 PR 的主要描述中以反應形式表示。從「:-1:」到「:+1:」的變更也應在註解中明確說明。
時程
- 高優先級問題的 PR 應在可用後立即進行程式碼審查。
- 里程碑中的問題的 PR 可以等待幾天。
- 如果 PR 上沒有「:+1:」,則 PR 建立者有責任與其他人聯繫並讓他們的程式碼接受審查。重申一下,PR 需要「:+1:」才能合併,如果尚未審查,則 PR 的開啟者有責任召集審查人員。
- 如果出現「:-1:」+ 沒有明確的解決方案,PR 的建立者和「:-1:」投票者都應計劃在接下來的一兩天內花一個小時討論問題,並計劃如何解決它。
- 如果在一周後帶有「:-1:」的 PR 沒有任何進展,@salsakran 將會介入。
如何提高程式碼審查的品質
有關程式碼審查研究的摘要,請查看 程式碼審查在現實世界中如何運作(以及不運作)。
PR 作者可以做些什麼來獲得更好的審查
- 透過註解程式碼的重要部分來引導審查人員。
- 如果您需要某人的專業知識/意見,請標記該人員。
- 透過使用 注意事項和警告 來增強 PR 描述 - 如果您希望某些資訊突出顯示,這些可能是有效的工具。
PR 審查人員可以做些什麼來提供更好的審查
- 密切注意測試檔案。請注意我們傾向於低估測試檔案的重要性,並有意識地努力徹底審查它們。
- 從對變更最重要的檔案開始,而不是從 PR 中按字母順序排列的第一個檔案開始。
- 如果您覺得自己缺乏背景知識,請要求作者提供更多詳細資訊/更好的描述。
- 除非變更微不足道,否則請查看該分支,並在本地使用包含變更的 Metabase。確保一切運作正常。
閱讀其他 Metabase 版本的文件。