作者:jon atack
來源:https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core
引言
審核(reviewing)和測試(testing),可能是給 Bitcoin Core 貢獻代碼的最佳起步方式。
豐富的審核和測試常常被長期的 Bitcoin Core 開發者提及,因為:
- 資源瓶頸,以及
- 它是學習、開始貢獻代碼和積累在社區中的剩餘的最佳方式,也是最有效益的方式。
在你開始之前
本指南建立在這些文獻的基礎上:
- 《向 Bitcoin Core 貢獻:個人經驗》,來自 John Newbery(2017)
- 《理解比特幣的技術一面》,來自 Pierre Rochard(2018)
- 硬核研討會 視頻 / 轉錄稿 / 幻燈片,來自 Jeremy Rubin(2018)
術語
ACK,NACK:它們的定義和起源可以在 這裡 和 這裡 找到。
Nit:一個微小的,通常不造成阻塞的問題。
PR:“代碼拉取請求(pull request)” 的縮寫,有時候也被稱為 “合併請求(merge request)”。它是對源代碼庫中的代碼或說明書的一次變更提議。
WIP:“還在開發中(work in progress)” 的縮寫。
通識
作為一名新人,我們的目標是嘗試增加價值,以友好和謙遜的態度,同時儘可能多學習。(雖然這些目標也不是隻對新人有意義;或者說,這個過程不會有終點。)
一個好辦法是,不把它當成一個由你主導的事,而是問自己: “如何最好地服務大家?”
新的貢獻者面對的最難問題之一是代碼庫的廣度以及圍繞它的技術複雜性。
要意識到還有你不知道的事。長期開發者有多年的開發經驗和專業背景。社區已經建立了知識和經驗的深度的集體財富。請記住,你的新想法可能已經有人提過、考慮過好幾次了。
請記住,貢獻者和維護者的時間精力都是有限的 —— 在你請求幫助的時候,應該謹慎並且尊重。目標是給予多於索求、幫助多於妨礙,同時加快速度。
多多嘗試自己搞明白,至少要充分尊重其他人的時間。
關注 #bitcoin-core-dev 即時聊天室以及 bitcoin-dev 郵件列表。
更多值得關注的 IRC 頻道可在此處找到。
在你開始之前,要先投入大量時間:
- 理解貢獻的流程和指引,不僅僅要閱讀代碼庫裡的說明書(尤其是《貢獻指南》、《開發者筆記》和《生產力筆記》這幾篇)(以及廣義地說,文檔 和 測試 代碼庫 中的一切),還要關注 #bitcoin-core-dev IRC 頻道中的互動,以及代碼庫中正在進行的代碼審核。
- 瞭解定期貢獻者:他們做些什麼、喜歡什麼、想要什麼,以及他們如何接收反饋。
許多新來者一上來就打開 PR ,但這只是在幾百個正在等待有價值審核的 PR 中增加一個。從審核已有的 PR 、理解什麼類型的 PR 和審核是更有幫助的、與此同時逐步理解全景,會好得多。
一個有用的經驗法則是:每當你要創建一個 PR,就先審核 5 ~ 15 個 PR 。
全景
全景(big picture)比 nit、拼寫和代碼風格都要重要得多。
全景審核有幾個不同層級:“這個變更會影響什麼行為嗎” 或者 “這個變更安全嗎” 與 “這是個好想法嗎” 是不一樣的。回答後面這個問題需要更多背景,這也是你會逐漸瞭解的事。別因為它而停止思考這些問題、也別放棄在這個層面上審核。
提升你對全景的理解的步驟:
- 完成 Chaincode Labs 的學習指南
- 考慮申請和參加最受推薦的 Chaincode Labs 在線課程(這個項目實際上也為搜尋正在找比特幣工作和開源獎金的新開發者服務)
- 學習比特幣升級提議(通常以單數和縮寫形式的 “BIP” 指代),並經常回顧
- 訂閱 Bitcoin Optech 週報並使用他們的 topics 頁面作為手邊資源
- 完成 “從命令行學習比特幣”
追求質量而非數量,並在深入的工作和快速出成果之間取得平衡。
撰寫說明有重要意義,比如說,關於各部件如何工作及如何交互的概要說明、清晰且準確的代碼文檔、一個函數是否有號的說明和 Doxygen 文檔、測試日誌(既包括 info 也包括 debug),等等。
測試覆蓋面也是重要的,別猶豫,去提升和編寫任何缺失的單元測試、功能測試以及模糊測試。
稱為一個謙虛、友善的貢獻者。通過審核、提出測試以及有用的修復措施、提議變更代碼初始狀態(rebase)來幫助 PR 前進,甚至,可以考慮在 PR 沉寂幾個月之後提出接管請求。總而言之,互幫互助!
NIT
嘗試避免過度糾結 PR 中的 nit、細枝末節和代碼風格,尤其是那些還打著 “WIP” 標籤的 PR;並且,在一個 PR 才剛剛創建、作者還在尋求概念上的 ACK(支持)和方法上的 ACK 的時候(比如還在尋求普遍共識的階段),不要急著挑刺。
長期貢獻者都報告說,這樣的活動讓他們反感;而且,這樣做會消耗你在項目中的社會資本。嘗試理解人們需要什麼樣的審核,以及什麼時候需要審核。
評論任何 nit 的最佳時機,都是在 PR 已經獲得足夠多 概念/方法 上的 ACK(也即得到共識)之後,在 PR 終結、獲得 “已測試 ACK” 之前。如 Pieter Wuille 在 IRC 中說的:“我感覺,對一個 PR 來說,最挫敗的事情就是得到了大量關於 細枝末節/nit/代碼風格 的評論,而依然不知道這個 PR 作為一個概念是不是可取的。”
給出對 nit 和代碼風格的建議時,應該友善、輕鬆、鼓勵 —— 比如,“請自由忽略”、“如果你恰好改變代碼,可以順帶調整”,等等。
請記住,沒有人有義務將你的審核意見考慮在內;如果作者認為你的意見已經超出了變更的範圍、因此回覆稱不想採納,那沒有任何問題(尤其是在你的評論都是在糾纏 nit 的時候)。
提高
在你能夠做到的時候,提高你審核的 PR 的難度和優先級。
審核的質量比數量重要得多。你可以通過為高優先級的 PR 和更難的 PR 提供深入、高質量的審核來學習以及增添更多價值。這些 PR 可能會嚇倒人們,可能會停滯幾個月,因為缺乏高質量的審核、遍佈吹毛求疵的代碼風格和 rebase 評論而讓作者的激情消磨殆盡。好好審核它們是給比特幣提供真正的服務。
提升的過程需要時間;沒有什麼能夠替代投入在瞭解背景、關注 代碼、問題報告、PR、#bitcoin-core-dev IRC 頻道和 bitcoin-dev 郵件組的經年累月時間。
在開始一次審核之前,一個有用的問題是:“在這個階段,它最需要的東西是什麼?” 回答這個問題需要經驗和積累起來的背景知識,但它的有用之處在於,可以確定如何以最少時間添加最大價值。取決於變更的複雜性和致命性,以及這個 PR 在審核流程中已經走過的階段,審核可能要略讀代碼、在關鍵位置的一個相關的代碼評論中應用大量背景知識,而不是運行包含 debug 開發、測試和審核每一次提交的完整審核。不過,在絕大多數情況下,完成合適的全面審核都是最好的、能夠增加最多價值。
一步一個腳印
把自我評價和願望排除在外。不要把事情變成個人意氣,應該推動事物前進。
有所懷疑的時候,假設他人有良好的意願。
對人們和結果保持耐心。
公開表揚;私下批評,並且應該帶有鼓勵。
持續幫助。每天都要努力。
所有事情都是說起來容易做起來難,寬恕你自己,也寬恕其他人。
請記住:每當你要創建 1 個 PR,就先審核 5 ~ 15 個 PR(或者 處理/測試 5 ~ 15 個問題報告)。
最後,要從不同背景、不同經驗層級的人瞭解你的貢獻的價值,而不僅僅是你的同事或熟人。聯繫新朋友(直接通過 IRC 消息挺好的),問問他們需要什麼幫助。你可以偶爾請求幫助,但不要把這當成一種權利。多給予,少索取。
技術細節
別相信,去驗證(Don’t trust, verify)。 儘可能減少你的審核流程對 GitHub 的以來。只為 GitHub 的元數據而使用 GitHub 網站,例如,閱讀評論和添加你自己的評論 —— 審核提交和代碼的工作應該在你本機環境中完成。
為本機拉取代碼
因此,審核過程的起點是為你的電腦拉取 PR 分支,從而在本機編譯和審核。相應於你的想法、需要、磁盤空間、互聯網帶寬,有不同的方法。以下僅舉幾例:
- 使用
git checkout pr/<number>來拉取遠端的 PR, 就像這篇短小精悍的 gist 說的那樣,可以修改成適合你的需要。 - 我的 git 配置的
[remote "origin"]部分:fetch = +refs/pull/*/head:refs/remotes/origin/pr/* - Bitcoin Core 貢獻者 Luke Dashjr 的版本:“為了避免所有的合併分支,將 origin-pull remote 配置成”:
fetch = +refs/pull/*/head:refs/remotes/origin-pull/*/head - Bitcoin Core 的文檔:使用 refspecs 輕鬆引用 PR
- 使用
pull/<number>/head(貢獻者分支)和pull/<number>/merge(合併到 master 分支), GitHub 會將 PR 暴露成上游代碼庫的分支,例如,git fetch origin pull/17283/head && git checkout FETCH_HEAD。也就是說,我傾向於儘可能少依賴 GitHub。
你可以在貢獻者分支上測試一個 PR,也可以在合併了變更的 master 分支上測試它。測試後者,可以檢查自上一次 PR 提交以來是否有什麼合併到 master 分支的東西打破了變更,非常有用。
然後,你可以開始在本機編譯和測試,同時開始閱讀代碼。你應該能夠熟練地從源代碼編譯 Bitcoin Core,然後運行單元測試和功能測試,因為你需要為許多 PR 做測試。因此,Bitcoin Core 的《生產力筆記》是無可替代的。
閱讀和了解 Bitcoin Core 的《開發者筆記》。
對比工具
在編譯和測試還在運行中時,開始在你本機環境中審核每一次代碼提交,使用 gitk、meld、meld for macOS、GNU ediff for Emacs、vimdiff 或 vim-diffconflicts for Vim、opendiff on macOS、diffoscope 這樣可以高亮差異部分的對比工具(這裡還有一份對比工具使用技巧)。
如果你使用 gitk 並且喜歡黑暗模式,我建議使用 Dracula for gitk 。
Git Grep
熟練地使用 git grep 來搜索代碼庫。你會一直使用它,直到在代碼庫中找到你需要的材料。在命令行環境中運行 git grep --help 來獲得幫助。
如果你不確定從哪裡開始
閱讀代碼、閱讀 PR 評論,然後回頭重新閱讀兩者。找出不理解的地方,然後搞清楚。如是不斷重複。
一旦所有一切都開始明朗,那就在 regtest/testnet 上運行 bitcoind (或者在主網上運行,只投入很少的錢),然後追蹤和搜索相關的日誌(運行 bitcoin-cli help logging 來獲得不同的 bitcoind 日誌類別以及如何 打開/關閉 它們)。
也許你可以添加一些自定義的日誌邏輯,LogPrintf 或者 assert; 將它們添加到別人的代碼中是一種特權(想了解為什麼,請在代碼庫中運行 git grep -ni logprintf 或 git grep asset)。
運行相關的功能測試,查看 debug 日誌。驗證它們在 master 分支上的出錯方式 符合預期。然後回到 PR 分支,逆轉或改變新的測試,讓它們失敗,並理解其中的原因。
也許 C++ 的 gdb 或 Python 的 pdb (或者在任何功能測試代碼中加入 import pdb:pdb.set_trace() )可以獲得斷點視圖。求值。運行 RPC 命令。
檢查該 PR 是否忽略了任何調用站點、頭文件或聲明。
嘗試重構代碼到更好狀態,並探索為什麼這樣行不通。做好實際花費時間是預計時間兩倍長的準備。沒錯,這就是工作。
也許可以運行 strace(man page strace)來跟蹤系統調用和信號。
取決於變更的內容,貢獻基準測試、內存分析/valgrind 或火焰圖(flame graphs)到 PR 審核中,有時候會很有幫助,甚至是決定性的。
技術資源
我為自己整理了一份包含多種技術筆記的文檔,我在開發 Bitcoin Core 時經常引用它們,放在這裡:jonatack/bitcoin-development/notes.txt 。這些筆記所在的代碼庫中還有有用的資料。另一個很好的資源庫是 fanquake/core-review 。
調試
兩個很好的 gist,講解了調試 Bitcoin Core :
添加缺失的測試
雖然你是在審核,但自己編寫測試可以幫助你理解代碼的動作並驗證變更的內容。而且,如果它們添加了有用的覆蓋面,你可以向作者提議在 PR 中加入這些測試。提議自動化測試是開始貢獻的非常有用的方式。有人審核代碼並提供額外的測試,作者會很感激的。這裡就有一個例子。
從全景到 nit
請記住,全景比 nit、拼寫和代碼風格都重要得多。請重新閱讀上文的 “NIT” 一節。在審核中,應嘗試避免評論這些東西,即使你沒有別的東西要評論。我知道,這很難 —— 我也很多次管不住自己 —— 但有一些更好的替代:
提問
作為一個審核員,(無需對代碼的專門知識)你可以做的一個好事情是 提出問題 。PR 的作者通常樂於討論自己的工作以及看到對它的興趣。所以,花上 20 多分鐘,看看變更的內容、找出看起來最令人困惑或者意外的地方,然後在 PR 評論中(或者 #bitcoin-core-dev 頻道中)禮貌地問問這些問題。可能其他人也對同樣的問題感到亦或,然後它可以得到更好的澄清和記錄。這樣一來,你既可以學到知識,又能幫助這個項目變得更加易於理解(本段要歸功於 Russ Yanofsky)。
同儕審核
確保你學習並理解了 Bitcoin Core 的同儕審核流程。這個流程 經常 更新,所以要經常回顧。
“ACK”(起源)一般用在審核員對自己如何審核以及手動測試的描述之後。作為一個新的貢獻者,建議在審核評論中更加詳細地提供關於你做了什麼、想了什麼的詳細描述,以證明你理解了這項變更。
“Concept ACK(概念 ACK)” 意味著審核人認可並同意這項變更的目標,但(尚且)沒有承認自己看了代碼、測試了代碼。這對 PR 作者來說也可能是一個有價值的信號,讓他們知道這個 PR 是有價值的,並且是往正確的方向前進。相對應的,“概念 NACK” 則表示不同意這個 PR 的目標。
“Approach ACK(方法 ACK)” 則比概念 ACK 更進一步,意味著既同意 PR 的目標,又同意這個 PR 用來實現這一變更的方法。“方法 NACK”則表示同意目標,但不同意實現的方法。
審核人有時候會評論 “代碼審核 ACK” 來表示代碼看起來不錯,但他們還沒測試變更的內容,或者對概念還沒有形成觀點。添加更多的背景會更好:“代碼審核 ACK HEAD,還不清楚概念的用意,我將驗證 x、y、z ”等等。
還有其它 ACK 變體:“tACK” 或者 “tested ACK”,表示已經測試;而 “utACK” 或者 “untested ACK” 表示尚未測試。
手動測試新特性,或者報告問題,都是受到歡迎的。審核評論中出現了這樣的字眼:“以下是我的測試內容和測試方法”,那是非常有幫助的,尤其是末尾還有一個 “ACK” 的話。
一些 PR 可能難以測試,或難以 ACK,因為它的複雜性、背景或缺乏測試和模擬的框架。比如說,如果你透徹審核了代碼,留下 “我看起來代碼是正確的,但對於它的動作,我無法自信到給出一個 ACK” 這樣的評論,那也是非常有用的貢獻。
還有其它有用的評論,比如, 對於包含了 move-only 的評論,“驗證了 move-only 部分” 就是有用的;還有 “苦苦思索改變 X 有沒有可能打破 Y,但沒有想出任何結果(這種情況真的會發生嗎?)”,等等。
帶有提交哈希值的 ACK
給出一個 ACK 的時候,通過附加 HEAD 提交(或者你審核的那一個提交)的哈希值來指明你審核的代碼狀態。免信任的、正確的方式是使用來自你的 本機 分支的哈希值,而不是來自 GitHub 網頁的哈希值。這樣一來,除非你的本機工具被攻破了,否則你可以保證,你是在 ACK 具體的一些變更。在強制推送(force push)發生、鏈接到的舊提交在 GitHub 上已經丟失時,這樣做也是有用的。
一個完整的 ACK 應該是這樣的:“ACK fa2f991 ,我編譯了、運行了測試,通過運行 X/Y/Z 手動進行了測試,並且審核了代碼,它看起來不錯,我同意它可以合併。”
當前的 Bitcoin Core 合併腳本會將(在合併的時刻)與 HEAD 提交相關的每一條 ACK 意見的第一段都複製到合併代碼的提交中。所以,請記住,你在其中寫下的任何東西都會被合併腳本複製,成為 git 歷史上永遠的一部分。
一個複雜或者更加高風險的 PR,可能需要至少 3 ~ 4 個有經驗的 ACK ,才能合併。
APACHE 投票系統
Bitcoin Core 的審核人們經常在評論中使用 Apache 投票系統。這裡就是一個例子。
對人寬容一點
審核代碼,而不是審核貢獻者本身和他們的評論。
當你不能同意的時候,一次性說明你的觀點,然後推進。這裡就是一個例子。不要用多次發言沖垮評論去,也別恐嚇他人或過度反應。要有耐心,不要過激或霸凌他人。請記住,最重要的東西可能不是討論中的問題,而是你跟其他貢獻者的關係。
作為一個新貢獻者,在給出 NACK 時要慎重。要默認假設是你對背景缺乏理解。如果你要給出 NACK,請提供好的推理。這裡就是一個例子。
使用 OpenTimeStamp 簽名提交
一些比特幣貢獻者會為 ACK 簽名並附加 OpenTimeStamp(Open 時間戳)。雖然這超出了本文的範圍,但使用 OpenTimeStamp 的 Git 插件來簽名你的提交是非常簡單的。
可摺疊的評論
一段時間以後,你會注意到,貢獻者們有時候會使用可摺疊的評論來審核。太酷了,你會想,這是怎麼做到的 ?它用的是 HTML 的 details 標籤。這裡有用法說明。
致謝
感謝 Steve Lee(moneyball)和 Michael Folkson 審核本文並提出他們的建議。
通過關注值得致謝的 Bitcoin Core 開發者,本文包含了在 GitHub 和 IRC 上觀察到的評論:Wladimir van der Laan、Marco Falke、Pieter Wuille、Gregory Maxwell、Anthony Towns 和 Russ Yanofsky 。
多年以來,我因為 BDFL(終身仁慈獨裁者)在編程語言和開源項目中的影響力而失望。Wladimir van der Laan 為比特幣貢獻的 長期 謙遜 的服務,點燃了我再次投身開源項目的可能性。
最後,衷心感謝各位 Bitcoin Core 貢獻者對我的審核的耐心,主要有 John Newbery、Marco Falke、João Barbosa、practicalswift、Gregory Sanders、Jonas Schnelli、Pieter Wuille 和 Wladimir van der Laan 。還要感謝 Adam Jonas 和 John Newbery 在一開始的指引和建議。
(完)




