關於 PR review 的兩、三事
The devil is in the details! 你是否曾經有想要幫忙 review 同事 PR 卻有些疑慮？像是這個改動會不會很難維護？這個是不是會有技術債？
今天這篇Notes主要為紀錄讀了 Google SWE 這本書的一些 PR review 心得與分享在書上提到 Google 工程師在 PR review 時會關注的焦點！
在 Google 一個 PR 要能順利進到 Master 至少需要符合三個條件，我想這和我們目前工作上的經驗差不多，分別是:
- 這個 PR 的變動需要和這個 PR 的 Owner 說明的實作相同
A correctness and comprehension check from another engineer that the code is appropriate and does what the author clamin it does.
- 需要被這個 code base 的其中一個 code owner 同意，因為擁有者通常是最清楚前後的改動會不會有什麼變化的人
Approval from one of the code owners that the code is appropriate for this particular part of the codebase.
- 須符合團隊的最佳實踐，確保這些改動符合團隊規範和風格的可讀性
Approval from someone with language "readability" that the code conforms to the language's style annd best practices, checking whether the code is written in the manner we expect.
Code Review Benefits
都需要經過這道手續？這個過程帶來的結果是短期的好處？還是長期的好處呢？
一個經過設計的 Code Review 流程和文化，可以帶來幾個好的價值：
- 檢查程式碼的正確性 (Correctness)
- 確保程式碼的改動可以讓其他工程師理解 (Comprehensible)
- 確保和程式庫代碼風格的一制性 (Consistnecy)
- Psychologically promotes team ownership
- 讓資訊跟知識能夠在不同成員之間傳遞 (Knowledge Sharing)
- 提供歷史紀錄讓程式碼能夠被重新檢視 (Provide a historical record of the code review itself)
Code Correctness
通常 Reviewer 只會關注程式碼是否有正確的測試、設計、功能是否正常和是否有效率。在這個階段儘管沒有相關的 Domain Knowledge 依然可以針對正確來進行 Review，避免有 Bug 的 Code 進到 Master，越早發現問題，所需修復的成本就越少，甚至可以省下更多的時間在測時、除錯及效能優化的問題上。
在此階段 Reviewer 可以有自己的主觀或客觀意見，如果建議的修改方式可以讓其他工程師讀懂或者有更進一步的效能優化。普遍來說工程師都鼓勵透過 PR review 來不斷優化和改進 Codebase，而不是等到有共識或找到最佳解，這樣有助於加速 Code review 的流程。
另外確保正確性並不是 Code Review 帶來的唯一好處，他只是確保了這些新增的改動能運作，並且能構隨著這個 Repo 或 Codebase 被改動之後，未來的人能夠輕易的解讀，不論是在任何時候
To evaluate those aspects, we need to look at factors other than whether the code is simply logically “correct” or understood
Comprehension of Code
讓除了作者之外的團隊成員能夠理解或從不同角度去 review code base 的改變，是非常重要的一環。在這個階段如果有其他的 reviewer 提出建議或不同的觀點， code owner 不一定需要因為這樣而去修改邏輯，但是呢，我們需要做更多個解釋或說明來幫助其他人能夠理解這段改動。
Code Correctness 和 Code Comprehension 是 LGTM 需要具備的兩個條件，當 Engineer 標記 PR 為 LGTM 這就表示這段 Code 的變動符合工單上的需求，同時也易於理解
Code Consistency
保持 Codebase 的一致性，更容易能夠讓其他團隊成員 review code，保持一致的風格，除了讓團隊成員更容易知道這些程式碼的意義之外，在和第三者或其他專家諮詢時，也更能夠專注在詢問的事情的邏輯上。
Psychological(心理上的) and Cultural Benefits
Code review 除了能建立團隊成員心理與文化上的認同外，總體而言，大多數的工程師都希望能夠自己能被挑戰，在 PR review 的過程中能獲得有用的建議或者是自己沒想到的問題。
另外一個心理上的好處是自己的實作能夠被驗證，進而多一份信心，就算是在有能力的工程師可能都會有被冒名頂替或被抄襲的問題，然而像是 Code review 這樣的過程能夠驗證或認證一個人對於工作的價值與產量。
Code review 不僅是對於自己有幫助之外，對於幫忙 review 的也相當有價值。隨著一個工程師在專業領域的成長，有時候他們很難發現正向的意見並且從中改進，因為自己已經是該領域的專家，但是仍有可能會在小地方犯錯，然而 Code review 就是能提供這樣的價值。
從 Code review 的過程當中，亦可以讓 Owner 能夠更小心的關注這些建議者提出的建議或要求修改的地方，是不是有任何的瑕疵。大部分的工程師都不是完美主義者，所以默認都會覺得只要可以 “ Gets the job done” 就好，與其花太多的時間在一開始精雕細琢，不如先求有。 正因為如此，一但我們沒有了 Code review 的過程，就有可能將有疑慮或有缺陷的程式碼進到代碼庫中，也為此可能留下所謂的技術債。
Code review 這個過程讓工程師在心理上有了強制性及被約束性，以確保他們在將他們的變更在交付前，必須審慎地檢視，然後再交由其他人二度檢驗。
Knowledge Sharing
知識的傳遞和分享是最常被低估的 Code review 帶來的好處之一，我們可以想像 Code review 的過程中，你所要求的 reviewer 是對於你當前這段代碼變更前，最熟悉的開發者，因此當你提交你的變更，這些專家能夠從更深或更進一步的角度知道，你可能在做代碼變更的時候沒有想到的事情，也因此再這樣與不同的人互動當中，我們可以無形中傳遞不同的資訊，比如一些程式的技巧或者是最新的規範，透過反覆的提問與回應互動，讓一些新的規範或模式，隨著時間與不同人之間的互動逐漸落實。
更重要的事，這些討論和變動的原因會成為代碼庫的一部分，日後如果發生了問題，可以很好的用來除錯或理解，在代碼變動的那個時間點下，是不是有人提除了問題跟為什麼要這樣修改。
再談談 Code Review 的最佳實踐
Be Polite and Professional
基於 Code comprehension 的原則，一個 PR review 至少要有一個工程師 review 並留下 LGTM，才表示整個過程完成，這中間不管有多少的 comments 或其他 review ，我們在提交變更之前，都需要確保所有的疑慮或問題都被修復才能夠提交。這一點很重要，我們必須確保所有的回饋和批評都能得到正確得改正或回應。
一般來說，當 reviewer 提出和 owner 不同的實踐方法或替代方案時，owner 可以依照團隊的規範選擇是否按照 reviewer 的實踐進行變更，反之如果有什麼缺現被發現在這個過程中，不管是 owner 或者是 reviewer 都能學到東西。
所有的評論都必須要保持嚴謹及專業，reviewer 必須要小心不要掉進 owner 的思維裡，最好是透過提問的方式來驗證，為什麼這段變更是這樣，而不要再提問或有疑慮的當下，就先假設對方的想法是錯誤的。
在 Google ，通常會被期待一個 PR review 的 comments 能夠在 24 小時內被回覆，此外如果一個 PR 沒辦法在 24 小時內被看完， Reviewer 需要在 PR 上面留下他們目前看到的地方，並且盡快的將還沒看完的地方看完。
Reviewer 應該要避免零碎的 review ，這樣對於 code owner 來說很困擾，這就好像已經改完了，又提出一些可能是不相關的修正或回饋，讓整個 review process 時間變長。
我們要保持專業
You are not your code, and that this change you propose is not “yours” but teams”
The responsibility of an author is to make sure that this code is understandable and maintainable for the future”
我們要將每一個 reviewer 提供的建議視為一個 TODO 項目，所有的評論與建議雖然不一定要被接受，但是他至少需要被妥適的回應或解決。如果你不同意 reviewer 的建議，你要回應並說出你的疑慮，並且不要自行 mark as resolved ，直到你們雙方覺得有更好的方案或共識。另外一種方式，是提出替代的方案，並留下 PTAL(Please take another look)，請 reviewer 再看看。
“Code review is a learning opportunity for both the reviewer and the author”
同樣的如果你是程式的擁有者，未來如果經過審查需要變更你當時的設計，你需要審視並且接受提出修改你程式碼的作者，意思就是你必須要尊重那些可以做的更好或能被修改的地方。
Write Small Changes
要讓 code review 的流程快速而敏捷，最好的方法就是保持每一次的變動很小，因為較小的變動比較容易理解。對於不管是 reviewe 或 owner 來說一個理想的的 Code review 需要盡量易於理解和保持單一個點。在 Google 的文化中，不鼓勵一次有太大規模的修改， reviewer 甚至可以因此拒絕這類的更改，因為這樣巨大的更改，不利於理解和審查。
此外，一次一點小的改動，同時也可以避免 owner 需要花太多時間去針對 reviewer 所提出的建議去修改，進而避免專案因為 review 而延宕的可能。
同樣的原理我們可以套用在專案的管理上，如果能夠盡可能的將需求、Bug 基於這樣的原則拆小，那麼我們就能夠維持暢流，更快的交付專案內容。(Narrow down)
“It’s important to acknowledge that a code review process that relies on small changes” 然而有時候如果是新的 feature 呢？巨大的改動是必須的，雖然小的變動容易被理解和消化，但是在更大的專案或流程中，組合起來會變的更難理解，所以在 Google 中也有一派的人認為變動不應該太小，因為沒辦法看到背後的全貌。
雖然我們可以透過 share branch 或一些技巧來管理這些比較大的變動，讓整個變動易於理解，但是這些東西常常可能會產生另外的問題(overhead)
“Consider the optimization for small changes just that: an optimization and allow your process to accommodate the occasional larger change”
盡可能的讓每一個 PR 的變動不要有太大的規模，但也需要適時的接受大規模的修正。
這裡指的 Small changes 指的是 200 行以內的改動，這樣的行數既不會太笨重影響的範疇也不會太大。在 Google 裏，他們會期待 reviewed 的過程能夠在一天中被完成，但這不表示整天都在 review PR 而是花小部份得時間每天提供初步的回饋和建議。
Write Good Change Descriptions
“A change description should indicate its type of change on the first line as summary”
雖然第一行很重要，但是具體的細節還有為什麼這麼做，還是需要清楚的寫在描述裡頭，“Bug fix”
這樣的訊息對於現在的 reviewer 或未來的 developer 來說並沒有什麼幫助，如果有不少的東西需要被條列在裡頭，那麼就條列式的把它列在描述中或者是 commit 裡頭，但是要注意保持簡潔扼要的原則！
探究 (Drilling down) 變動、了解變動的原因，對於 debug 來說非常有幫助，也因次我們會每次透過 git history 去理解，誰改了什麼做了什麼，在什麼單號上！
i.e. feat: PD-12345 a meaningful description
- add …
- add …
- improve …
如果 Code reviewer 不能理解你為什麼做這個改動，儘管這個改動是正確的，這也表示這樣的程式結構或描述需要被更新或解釋。如果再 PR review 中，有新的實踐或想法決定被採用，我們需要更新描述來反應這個事實，或者將討論的過程與記錄留在 PR review 中。
A code review is not just something that you do in the present time; it is something you do to record what you did for future(posterity).
Keep Reviewers to a Minimum
Code review 這個過程是被建立於對工程師的信任以及專業。所想出來的一個過程。當然在某些情況下，讓不同的人去審視同一個 PR 肯定是有用的，但是儘管在那些情況下，這些 reviewer 要專注的應該也只是同一個更改，並從不同方面去審視這個變更。
Automate Where Possible
coding style, linter, prettier