Pull Request 審查流程
備註
本頁旨在說明我們期望的 Pull Request(PR)審查流程。主要讀者是負責審查與核准 PR 的引擎維護者。不過,許多內容同樣適用於希望了解如何確保自己 PR 能夠合併的潛在貢獻者。
從整體流程來看,Pull Request 的理想生命週期如下:
本文件將更詳細說明步驟 2、3、4 和 5。若需更完整的 PR 工作流程說明,請參閱 拉取請求工作流程文件。
備註
實務上,這些步驟可能同時進行。維護者常常會同時針對程式碼風格與品質給予意見,並一併核准 PR。
通常,Pull Request 的第一步是引擎維護者指派標籤,並標記給熟悉該程式碼區域的人員進行審查。
引擎維護者是指在 GitHub Godot 專案儲存庫內的「成員」及/或於 Godot 網站 團隊頁面 上所列的人員。維護者負責引擎的特定區塊。一般來說,他們擁有較高信任度,可以核准及推薦 PR 合併。
即使不是維護者,您仍然可以協助:審查程式碼、對 PR 提出回饋,並在本機測試 PR 以確認其運作符合預期。許多現任維護者在成為維護者之前,都是這樣開始貢獻的。
程式碼審查與測試
以下是貢獻者與引擎維護者在進行實質程式碼審查時可執行的事項。
備註
如果您希望進行程式碼審查,但無法完成清單上的所有項目,請在審查留言中註明。例如,即使您無法在本機編譯並測試該 PR,僅針對程式碼本身給予意見也相當有幫助。請放心進行審查,只要在最後註明您僅檢查了程式碼,尚未在本地測試變更即可。
1. 確認問題存在
每個 PR 都必須解決一個具體問題,且該問題需有記錄。請確認該 PR 有連結並關閉(或至少處理)一個錯誤或建議。如果沒有,請考慮要求貢獻者補充 PR 開頭說明,詳細描述該 PR 旨在解決的問題。
備註
在合併前,必須清楚說明_為什麼_需要這個 PR。這有助於審查者判斷 PR 是否如其所述解決問題,也能讓日後的貢獻者理解程式碼這樣設計的原因。
2. 測試 PR 並檢查是否有回歸(Regression)
即使有嚴格的程式碼審查與 CI,難免會有錯誤,有時貢獻者的修正同時也產生了新問題。維護者應避免合併出現 Regression 的程式碼,即使該 PR 原本解決了既有問題。
審查 PR 時,請確認它確實完成所述目標(例如修正指定錯誤或實作新功能),而且不會導致目標範圍外的其他功能出現問題。您可以開啟編輯器,嘗試常見功能(新增場景物件、執行 GDScript、開關選單等)來驗證。此外,也請特別留意程式碼中是否有引擎其他部分被意外更動,有時變基(rebase)過程中可能會夾帶不預期的修改。
3. 進行程式碼審查
程式碼審查通常由熟悉該領域的人進行,他們能夠提供讓程式碼更快、更有條理或更貼近慣用寫法的建議。不過,即使您經驗尚淺,也可以針對自己有把握的範圍進行審查並給予回饋。這不僅對維護者有幫助(多一雙眼睛總是好的),對您自己而言也能加深對該區域程式碼的了解,看見他人如何解決問題。事實上,閱讀資深維護者的程式碼,是認識整個程式碼庫的好方法。
進行程式碼審查時,請特別留意以下事項:
程式碼僅影響 PR(及其提交說明)所描述的區域。
審查時看到順手可修的小錯誤很容易就順便修正,但這樣會讓 PR 變得難以審查,也增加日後追查歷史紀錄的困難。相關區域的小幅修正可以接受,但若發現與此 PR 無關的 Bug,建議另開新 PR 處理。
程式碼確實遵循 Godot 的 API 與設計模式。
一致性很重要,優先採用程式庫中現有的解法,而非臨時針對個案的寫法。
這項修改是否影響到核心區域?
有時,原本只想修某個小問題,卻不小心影響到系統其他區塊。通常最好讓變更只限於原問題區域。如果覺得解法會牽涉到其他區域,建議先詢問團隊領導者意見,也許他們有不同的處理方式。
4. 與貢獻者反覆修正並改進 PR
維護者若發現程式碼有需要調整之處,應提供回饋與改進建議。建議依重要性排序:先處理整體設計與解法,再確認程式碼是否符合引擎最佳實踐,最後才進行 程式碼風格審查。
備註
在審查流程初期就明確溝通合併的阻礙。
若 PR 存在明顯阻礙或因其他原因不太可能被合併,請儘早明確告知。我們不希望讓人空等,只因為不敢早說「很抱歉,這不行」。
審查 PR 時,請謹記 Godot 行為準則 ,特別注意以下幾點:
請始終保持禮貌,友善且尊重他人。
請永遠假設他人出於善意。
任何回饋都非常歡迎,但請保持批評具建設性。
在與貢獻者反覆修正 PR 時,請避免以下情況:
不必要的多次重複審查。
也就是說,盡可能一次性完整審查 PR,避免多次回來重複指出第一次就能發現的問題。雖然有時難以避免,但還是要盡量一次性檢查清楚。
過度吹毛求疵。
程式碼品質標準可視所屬引擎區域彈性調整。一般來說,核心區域及效能敏感區的品質標準遠高於編輯器本身的程式碼。
擴大 PR 範圍。
提供相關背景、類似問題或建議有助於討論,但要求「順便把那邊也修掉」或「能不能也加上這個」對貢獻者並不公平。請自行判斷額外修正是否屬於本 PR 範圍,盡量讓內容聚焦於原始 PR。
最重要的是,不必一個人獨自處理 PR。可以隨時在 Godot 貢獻者聊天室 的適當頻道或 #general 尋求協助。其他團隊可能已標記待審查,也可以等候或主動請求他們幫忙。
5. 核准 PR
審查完畢,若認為程式碼已可合併進引擎,請點選「核准」。同時請留言說明您的審查內容(例如:是否有在本機執行、是否檢查了風格與正確性等)。即使不是引擎維護者,通過 PR 也能讓其他人知道這份程式碼有品質、確實解決了 PR 所述問題。非維護者的核准不保證一定會合併,仍會有其他人再審查,所以不必害羞。
程式碼風格審查
一般來說,我們會先進行程式碼邏輯審查,再進行風格與可讀性審查,因為貢獻者通常希望先確認整體思路可行,再花力氣微調風格。換句話說,維護者不該要求貢獻者修改可能還需大幅重寫的程式碼風格。同樣,PR 尚未審查前,應避免要求貢獻者 rebase。
話雖如此,不是每個人都能對程式碼正確性下判斷,因此在進行更深入審查前,先給予風格與可讀性意見也是完全可以且非常歡迎的。
實務上,程式碼風格審查可以與邏輯審查同時進行。重點是,合併 PR 前,必須同時審查其功能與風格。
審查程式碼風格時,請特別確認 PR 是否有遵循 程式碼樣式方針。雖然 clang-format 和各種 CI 檢查能抓出許多不一致之處,但仍有盲點。例如,您應檢查:
是否正確包含標頭檔案。
識別名稱是否採用
snake_case並遵循命名慣例。方法參數是否以
p_*或r_*開頭(用於傳回值時)。大括號是否正確使用,即便是單行條件式也要加上。
程式碼間距是否適當(方法間僅有一行空白,方法內容內不應有多餘空白行)。
備註
這個清單並不完整,詳細規則請參照連結的風格指南。請注意,clang-format 有它的侷限,無法抓出所有問題,請多加留意並累積經驗。
合併 Pull Request
一般來說,只有生產團隊成員或各區塊負責的團隊領導者,才能合併屬於自己負責範圍的 Pull Request。例如,網路團隊領導者可以合併僅變更網路相關、不影響其他區塊的 PR。
實務上,建議由生產團隊成員來負責合併 PR,因為他們比較熟悉整體程式碼庫,能判斷此 PR 是否會與近期或即將進來的變更衝突(或有其他暫緩合併的理由)。如果您認為 PR 已準備好合併,也可以留言提醒。
以下是在合併 Pull Request 前需進行的步驟。對於簡單直接的 PR,可以彈性處理,但遇到複雜或高風險的 PR 時,請務必嚴格遵循。
作為貢獻者,您也可以自行完成部分步驟,加速 PR 推進。
1. 徵詢正確人員/團隊的回饋
生產團隊成員應確保相關領域的適當人員有審查該 PR。部分情況需多位人員共同評估,有些則只需一名專業維護者核准即可。
通常不要僅靠一人(尤其是自己本身)的審查就合併 PR,建議徵詢另一位維護者意見,並確保所有受影響的團隊都有人參與審查。例如,若 PR 涉及文件,讓負責該功能區域的維護者檢查內容正確性,讓文件維護者檢查格式、語法與風格都很重要。
一個好的原則是,至少有一位專業領域維護者核准其功能正確性,另有一位維護者核准其程式碼風格。這兩者皆可負責最終合併。
務必確認審查與核准來自該功能區域的專業維護者。有時即使是資深成員,也可能因不熟悉該領域而留下一般性審查意見。
備註
要找出已準備好合併的 PR,可以篩選「已核准」並按「最近更新」排序。在主 Godot 儲存庫可使用 這個連結 。
2. 徵詢社群回饋
若 PR 難以吸引審查者,建議擴大範圍主動徵求協助,可詢問:
回報此錯誤的人,詢問此 PR 是否已解決他們的問題,
最近編輯過該檔案的貢獻者,是否能幫忙看一下,或
其他領域較有經驗的維護者,是否能給予意見。
3. Git 注意事項
確保 PR 僅有一個提交(commit)。
若每個提交都自成一體、可單獨構建出可運作的引擎版本,合併多提交 PR 亦可接受。但一般來說,所有 PR 建議僅保留一個提交,以維持 Git 歷史清晰。
審查期間的修正需 Squash 合併到主提交。
對於多提交的 PR,請確認修正已合併到正確的提交中,而非僅在最上層再補一個修正。
確保 PR 沒有合併衝突。
貢獻者可能需要將更動 rebase 到適當分支(如
master或3.x),並手動解決合併衝突。即使未顯示有衝突,特別是舊 PR,還是建議 rebase 一次,因為 GitHub 的衝突檢查不一定能抓到所有問題,CI 流程也可能已更新。檢查提交(commit)歸屬正確。
若貢獻者的提交簽名未與其 GitHub 帳號綁定,GitHub 將無法將合併後的 PR 正確連結至他們帳號,如此他們在貢獻歷史中無法獲得正確歸屬,UI 也會將其視為新貢獻者。
最終是否要修正取決於使用者自己,他們可以用與 GitHub 帳戶相同的 email 簽署提交,或將該 email 加入 GitHub 個人檔案。
檢查提交訊息是否合適。
雖然我們對提交訊息沒有硬性規範,但仍需言簡意賅、具描述性,並使用正確英文。維護者應該很熟練,通用範例如:「Fix <issue> in <part of codebase>」。更詳細建議請見主儲存庫的 contributing.md。
4. GitHub 注意事項
確認 PR 目標分支。
大多數 Godot 開發都以
master分支為主,因此大部分 PR 都應該針對該分支提出,然後再視需要回溯合併到其他分支。需留意有人會針對自己使用的版本(如3.3)發 PR,應指引他們改為針對更高階分支(如3.x)。若變更不適用於master,可直接針對目前維護的分支(如3.x)發 PR。針對多個目標分支各自開 PR 也是可以的,尤其當變更難以直接回溯時。也可進行 cherry-pick,若適用請加上相對應的標籤(如cherrypick:3.x)。
備註
已送出的 PR 可以變更目標分支,但要注意後果。這種變更無法與推送同步,會導致所有維護者都被標記需要審查,也可能造成 CI 無法正常運作。推送更新後通常能解決,但若問題持續,建議直接重開新 PR。
確認已指派正確的里程碑。
這樣能讓大家清楚該 PR 若合併,會納入哪個版本。請注意,里程碑僅供參考,無法保證該版本一定會包含此 PR。若 PR 未能及時合併,里程碑會被調整(PR 本身也可能需要更換目標分支)。
同理,若合併 PR 時其里程碑高於目前版本,或設為「*」(如「4.x」),請記得將其更新為目前版本。
確認 PR 開頭訊息包含「Closes #...」或「Fixes #...」等關鍵字。
這能將 PR 與 Issue 連結,合併時 GitHub 會自動關閉對應 Issue。注意:僅針對
master分支的 PR 可自動關閉,其他分支需手動關閉 Issue。請留言註明 「Fixed by #...」 或 「Resolved by #...」,方便未來查閱。為 PR 關閉的 Issue 加上對應的最近里程碑。
換句話說,若 PR 目標是
master,但之後有 cherry-pick 到3.x,則對應 Issue 的里程碑應設為下一個3.x發行版本。
5. 合併 PR
如果您身分合適(如生產團隊成員或該區塊負責人),且確認 PR 已充分審查,經上述步驟即可進行合併。