檢閱貢獻#
原則#
Arrow 是一個基礎專案,需要經過多年甚至數十年的發展,同時服務可能數百萬的使用者。我們相信,在審閱時一絲不苟,比寬鬆並追求快速合併更能為專案帶來更大的回報。
像這樣的程式碼審閱可以帶來更高品質的程式碼,更多人參與並理解正在變更的程式碼,以及一個總體上更健康、更有成長空間和適應新興需求的專案。
指南#
元資訊#
運用您自己的判斷。這些指南並非硬性規定。提交者應對其工作領域有足夠的專業知識,能夠根據他們的任何疑慮調整他們的方法。
這些指南並未按特定順序排列,也不打算用作檢查清單。
最後,這些指南並非詳盡無遺。
範圍與完整性#
我們的一般政策是不引入回歸錯誤或合併需要後續才能正確運作的 PR(儘管可以對此例外)。在合併後進行必要的變更,對於貢獻者和審閱者來說成本更高,對於其他開發人員來說也是如此,如果他們遇到合併的 PR 引入的問題,可能會感到困惑。
PR 的範圍內有哪些變更,以及哪些變更可能/可以/應該超出範圍並建立後續議題,應由作者和審閱者協商決定。
當要貢獻大量功能並且似乎希望逐步整合時,在決定如何劃分變更時,應優先考慮功能上的凝聚力(例如,如果要貢獻檔案系統實作,第一個 PR 可以貢獻目錄元資料操作,第二個 PR 檔案讀取功能,第三個 PR 檔案寫入功能)。
公開 API 設計#
公開 API 應引導使用者朝向最理想的建構。換句話說,如果有「最佳」方法來做某事,那麼理想情況下,它也應該是最容易發現的,並且輸入起來最簡潔。例如,安全 API 應比不安全 API 更突出,後者可能會在無效輸入時崩潰或靜默地產生錯誤結果。
公開 API 理想情況下應趨於產生可讀的程式碼。一個例子是當預期隨著時間的推移添加多個選項時:最好嘗試邏輯地組織選項,而不是將它們全部並列在函式的簽名中(例如,請參閱 C++ 和 Python 中的 CSV 讀取 API)。
命名很重要。試著問問自己,呼叫新 API 的程式碼是否可以在不必閱讀 API 文件的情況下被理解。應避免含糊不清的命名;不準確的命名更糟糕,因為它可能會誤導讀者並導致錯誤的使用者程式碼。
請注意術語。每個專案都有(明確或不明確地)設定關於如何命名重要概念的慣例;偏離這些慣例會增加貢獻者和專案使用者的認知負擔。相反,將一個眾所周知的術語用於與平常不同的目的,也可能增加認知負擔,並使開發人員的生活更加困難。
如果您不確定 API 是否適合手邊的任務,建議將其標記為實驗性的,以便使用者知道它可能會隨著時間而改變,同時貢獻者也不太擔心帶來破壞性變更的改進。但是,實驗性 API 不應用作迴避基本 API 設計原則的藉口。
穩健性#
Arrow 是一組開源函式庫,將在非常廣泛的環境中使用(包括在 Jupyter 直譯器提示符下擺弄刻意的人工資料)。如果您正在編寫公開 API,請確保它不會在不尋常(但有效)的輸入上崩潰或產生未定義的行為。
當實作非平凡演算法時,防禦性編碼可能有用,以捕捉潛在的問題(例如僅限於除錯的斷言,如果語言允許它們)。
攝取可能不受信任資料(例如磁碟上的檔案格式)的 API,應盡量避免在向其饋送無效或損壞的資料時崩潰或產生靜默錯誤。這可能需要大量的謹慎,這超出了常規程式碼審閱的範圍(例如設定 模糊測試),但仍可以在程式碼審閱階段建議基本檢查。
當呼叫外部 API,尤其是系統函式或處理輸入/輸出的 API 時,請檢查錯誤並傳播它們(如果語言不會自動傳播錯誤,例如 C++)。
效能#
考慮效能,但不要過於執著。如果輸入大小可能「很大」(大的含義取決於上下文:運用您自己的專業知識來決定!),則演算法複雜度很重要。在效能敏感的功能上,將效能提高 20% 或更多的微優化也可能有用;較小的微優化可能不值得花費時間,尤其是當它們導致更複雜的程式碼時。
如果效能很重要,請測量它。不要滿足於猜測和直覺(這可能基於關於編譯器或硬體的錯誤假設)。
另請參閱
除非真的很重要,否則盡量避免試圖透過以某種方式編寫程式碼來欺騙編譯器/直譯器/執行時期。這些技巧通常很脆弱,並且依賴於可能過時的平台細節,它們可能會使程式碼更難維護(一個可能阻礙貢獻者的常見問題是「對於我的變更想要移除的這個奇怪的駭客技巧,我應該怎麼辦?」)。
避免粗糙邊緣或退化行為(例如,當大小估計不準確地大時的記憶體膨脹)可能比試圖將常見情況提高一小部分更重要。
文件#
這些指南理想情況下應同時適用於散文文件和程式碼內的文件字串。
尋找含糊不清/資訊量不足的措辭。例如,「如果...則會發生錯誤」不如 「如果...則會引發錯誤」 或 「如果...則行為未定義」 更具資訊性(第一種措辭沒有告訴讀者在這種錯誤中實際發生了什麼)。
在審閱文件變更(或一般散文片段)時,請注意拼字、文法、表達和簡潔。清晰的溝通對於與來自廣泛背景的人們進行有效協作至關重要,並有助於改進文件。
有些貢獻者的母語不是英語(也許您也不是)。建議幫助他們,和/或在需要時尋求外部幫助。
交叉連結增加了文件的整體價值。Sphinx 尤其具有出色的交叉連結功能(包括主題參考、詞彙表術語、API 參考),因此請務必加以利用!
測試#
當新增 API 時,所有名義案例都應有測試案例。函式是否允許空值?那麼應該測試空值(當然,也要同時測試非空值)。函式是否允許不同的輸入類型?等等。
如果功能的某些方面很微妙(無論是根據定義還是作為實作細節),都應該進行測試。
應測試邊緣情況,尤其是在 C++ 等低階實作語言中。範例:空陣列、零區塊陣列、僅包含空值的陣列等。
壓力測試可能很有用,例如,如果正在新增非平凡的平行化,則可以發現同步錯誤,或者針對緩慢且直接的參考實作驗證計算論證。
然而,一個需要減輕的疑慮是執行測試套件的總體成本。持續整合 (CI) 執行時間可能非常長,我們應該警惕將它們增加太多。有時,微調測試參數以平衡測試的實用性與執行它們的成本是值得的(尤其是在涉及壓力測試時,因為它們往往意味著在大型資料集上執行)。
標籤#
在審閱 PR 時,我們應該嘗試確定相應的議題是否需要標記以下一個或兩個議題標籤
重大修復:變更修復了以下任一項:(a) 安全漏洞;(b) 導致產生不正確或無效資料的錯誤;或 (c) 導致崩潰的錯誤(同時 API 合約得到維護)。這旨在標記可能在使用者不知情的情況下影響使用者的問題的修復。因此,修復導致錯誤的錯誤不算在內,因為這些錯誤通常很明顯。導致崩潰的錯誤被認為是重大的,因為它們可能是阻斷服務攻擊的載體。
破壞性變更:變更破壞了公開 API 中的向後相容性。對於 C++ 中的變更,這不包括僅僅破壞 ABI 相容性的變更,但我們保證 ABI 相容性的少數地方(例如 C 資料介面)除外。實驗性 API 並非 免除此項;它們只是更可能與此標籤相關聯。
破壞性變更和重大修復是分開的:破壞性變更會更改 API 合約,而重大修復會使實作與現有的 API 合約保持一致。例如,修復導致 Parquet 讀取器跳過包含數字 42 的行的錯誤是一個重大修復,但不是破壞性變更,因為行跳過並非合理使用者會依賴的行為。
這些標籤在發佈中用於突出顯示使用者在考慮升級函式庫版本時應注意的變更。破壞性變更有助於識別使用者可能希望等到他們有時間調整程式碼後再升級的原因,而重大修復則突出顯示不升級的風險。
此外,我們使用以下標籤來指示優先順序
優先順序:阻斷:表示變更必須在下次發佈之前合併。這包括對測試或封裝失敗的修復,這些失敗會阻止發佈成功完成最終封裝或驗證。
優先順序:重大:表示高優先順序的議題。這是標記為「重大修復」的議題的超集,因為它也包含對導致錯誤和崩潰的某些問題的修復。
協作者#
協作者角色允許使用者被賦予分類角色,以便能夠幫助分類議題。該角色允許使用者標記和分配議題。
使用者可以要求成為協作者,或者當他們在專案中協作一段時間後,可以由其他社群成員提名。協作可以但不限於:建立 PR、回答問題、建立議題、審閱 PR 等等。
為了提名某人作為協作者,您必須建立一個 PR,將使用者添加到 .asf.yaml 上的協作者清單中。提交者可以審閱使用者過去的協作並批准。
長時間不活躍的協作者可以從協作者清單中移除。
社交方面#
審閱是貢獻者和審閱者之間的溝通。避免讓問題或評論長時間未得到回答(「長時間」當然非常主觀,但兩週可能是一個合理的經驗法則)。如果您近期無法分配時間,請明確說明。如果您沒有問題的答案,請明確說明。說 「我沒有時間立即處理,但我稍後會回來,如果我似乎忘記了,請隨時 ping 我」 或 「抱歉,我對此不熟悉」 總是比什麼都不說,讓對方感到疑惑要好。
如果您認識有能力協助解決阻礙性問題的人,並且過去的經驗表明他們可能願意這樣做,請隨時將他們加入討論(例如,透過輕輕 ping 他們的 GitHub 帳號)。
如果貢獻者已停止提供回饋或更新他們的 PR,也許他們不再感興趣,但也許他們也陷入了某些問題,並且感到無法進一步推動他們的貢獻。不要猶豫去詢問(「我看到這個 PR 最近沒有任何更新,您是否遇到什麼困難?您需要任何幫助嗎?」)。
如果貢獻確實是理想的,並且貢獻者沒有取得任何進展,也可以接手。然而,出於禮貌,最好先詢問貢獻者。
有些貢獻者正在尋找針對特定問題的快速修復,並且不想花費太多時間在上面。另一方面,有些人渴望學習和改進他們的貢獻,使其符合專案的標準。後一類貢獻者尤其有價值,因為他們可能會成為專案的長期貢獻者,甚至是提交者。
當向某些貢獻者指出問題時,他們可能會回應 「我稍後會修復它,我們可以先合併嗎?」。不幸的是,稍後的 PR 中是否真的會很快貢獻修復程式碼是很難預測或強制執行的。如果貢獻者先前已證明他們是可靠的,則可以接受按照建議執行。否則,最好拒絕該建議。
如果 PR 除了瑣碎或無爭議的問題外,通常已準備好合併,則審閱者可以決定自行將變更推送到 PR,而不是要求貢獻者進行變更。
理想情況下,貢獻程式碼應該是一個有益的過程。當然,情況並非總是如此,但我們應該努力減少貢獻者的挫折感,同時牢記上述問題。
與任何溝通一樣,程式碼審閱受 Apache 行為準則 的約束。這適用於審閱者和貢獻者。