Pull Request 審查流程

備註

本頁旨在說明我們期望的 Pull Request(PR)審查流程。主要讀者是負責審查與核准 PR 的引擎維護者。不過,許多內容同樣適用於希望了解如何確保自己 PR 能夠合併的潛在貢獻者。

從整體流程來看,Pull Request 的理想生命週期如下:

  1. 貢獻者開啟修復特定問題的 PR(最佳情況下同時關閉 GitHub issue 或實作 proposal)。

  2. 其他貢獻者對此 PR 提供回饋(包括進行審查和/或視情況給予核准)。

  3. 引擎維護者審查程式碼,並依需要提供回饋、請求修改或核准該 PR。

  4. 另一位維護者會著重於程式碼風格與可讀性進行審查,滿意後予以核准。

  5. 若團隊領導者或 生產團隊 成員認為該 PR 已經充分審查,則會執行合併。

本文件將更詳細說明步驟 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 到適當分支(如 master3.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 已充分審查,經上述步驟即可進行合併。