11.27 Code review流程規範-Git Flow+Pull Request

一、為什麼要code review

需要有Code review環節,流程規範

為什麼要Code Review?

希望能提高代碼質量,提高團隊技術,減少bug。

二、code review常用流程規則

比較常用的有:Git Flow+Pull Request(PR)

Code review流程規範-Git Flow+Pull Request

git flow

Code review流程規範-Git Flow+Pull Request

Pull Request(PR) 是你沒有權限往一個特定的倉庫或分支提交代碼,你請求有權限的人把你提交的代碼從你的倉庫或分支合併到指定的倉庫或分支。

由於PR需要有權限的人確認,所以適合在這個過程中做Code Review。

在支持PR模式的軟件裡,每一個PR都有一個新增代碼的對比(diff)界面。

代碼審核者可以在線瀏覽請求合併的新增代碼,並針對代碼行添加評論。

評論可以被所有有權限查看倉庫的人看到,每個人都可以回覆任何人的評論,有點像論壇裡某個帖子的討論。

這種模式是事後審核,也就是代碼已經提交到了中心倉庫,Review過程中頻繁的改動會造成歷史簽入記錄的混亂。

由於Git太靈活了,因此誕生了很多的Git流程,用來規範Git的使用。

常見的有集中式工作流、功能分支工作流、Gitflow工作流、Forking工作流、Github工作流。

我們對Git Flow做了些調整,調整後的流程被命名為Baza Flow,定義見後文。

根據Baza Flow,我們大部分倉庫只定義了2個主幹分支,master和develop。(例外,我們有一個倉庫有3個開發小組同時進行開發,定義了4個主幹分支,目前還比較順暢,再多估計主幹分支之間的合併就比較繁瑣了。)

master對應生產環境代碼,所有面向生產環境的發佈來源都是master分支的代碼。develop則對應本地測試環境的代碼。

絕大多數情況下,QA(測試)只測試develop分支和master分支的代碼。

由於開發人員都在一個團隊內,所以我們沒有采用基於倉庫的PR,採用的是基於分支的PR。

我們對主幹分支的操作權限做了限制,只有特定的人才能操作,develop分支是項目開發Leader和架構師,master分支是QA。

有權限往主幹分支合併的成員會按照約定的規則來執行合併,不會合並沒有完成審核的PR。

上面這點其實蠻重要的,所以我們會對有權限合併的人有特別的約定,在什麼情況下才能合併代碼。(見後文PR的說明)

PR的發起人要主動的推動PR的審核,Leader也會密切關注PR審核的進度,在需要的時候及時介入。

我們配置了CI服務器(什麼是CI)只編譯特定的分支,通常是develop和master分支。

所有的代碼合併到了主幹分支之後,都會自動觸發編譯和本地測試環境的發佈,QA無需依賴開發人員編譯的代碼來測試,也無需自己手工操作這些,保證了開發人員和測試人員的相互獨立。

我們本地測試環境的發佈包含了數據庫和站點的發佈,全自動的,發佈完成以後就是一個可用的產品,有時間這部分也可以分享一下。

我們還使用了Scrum裡面一個很重要的概念:完成定義。

就是我們規定了我們一個任務的完成被定義為:代碼編寫完成,經過自測,提交的PR經過審核並且合併到主幹分支。

也就是說,所有的代碼被合併到了主幹分支之後任務才算是完成,而被合併到主幹分支必須要經過Code Review,這是強制的。

三、谷歌的code review

谷歌code review開源地址: https://github.com/google/eng-practices

中文相關:https://xindoo.github.io/eng-practices-cn/review/reviewer/

https://xindoo.github.io/eng-practices-cn/review/developer/

審核者指南

開發者應當不斷改進 ,否則代碼庫整體質量也不會提升。

審查者的責任是讓代碼庫越來越健康,但是要有度,不能要求太苛刻。

代碼對系統有明顯的提升且正常工作,即便不完美,評審者也應該傾向於通過這次變更。

評審者應該追求 持續提高 ,而不是追求完美。

原則

  • 技術和數據高於意見和個人偏好。
  • 關於風格問題, 風格指南是絕對的權威。任何不在指南中指出的樣式(比如空格等)都是個人偏好的問題。
  • 軟件設計是有時候也會有多種有效的選項,如果開發者能證明(通過數據或者原理)這些方法都同樣有效,那麼評審者應該接受作者的偏好,否則應該遵從軟件設計標準。
  • 如果沒有其他的規則使用,只要保證不會影響系統的健康度,評審者可以要求開發者保持和現有的代碼庫一致。

解決代碼衝突

如果Code Review中有任何衝突,當很難達成一致時,開發者和評審者不應該在Code Review評論裡解決衝突,而是應該召開面對面會議或者找個權威的人來協商。

如果這樣都解決不了問題,那解決問題的方式就應該升級了。通常的方式是拉著團隊一起討論、讓團隊主管來權衡、參考代碼維護者的意見,或者讓管理層來決定。不要因為開發者和評審者不能達成一致而把變更一直放在那裡。

亮點

如果你看到變更中做的好的地方,告訴開發者做的很棒,尤其是當他們用某種很精巧的方式解決你評論中提到的問題時更應不吝讚美。Code Review過多關注於錯誤,但你也應該為開發者展現出好的一面點贊。

禮貌尊重開發人員

給予指導

接受解釋

縮寫解釋:

  • CL: “changelist"的縮寫,代表已經進入版本控制軟件或者正在進行代碼評審的變更。其他組織經常稱為"change"或者"patch”。
  • LGTM: "Looks Good to Me."的縮寫,看起來不錯,評審者批准CL時會這麼說。
  • 開發者指南

    一個CL描述是做了什麼更改以及為什麼的公開記錄。

    要言簡意賅。

    不要摻雜個人情感

    如果評審者批評了你的代碼,可以理解為他們在幫你、幫整個代碼庫、甚至是幫整個公司,而不是攻擊你或者是質疑你的能力。

    修復代碼

    添加註釋來解釋清楚代碼的邏輯。如果評論似乎毫無意義,那麼您的答覆應該只是代碼查看工具中的解釋。

    自我反思

    Code review流程規範-Git Flow+Pull Request


    分享到:


    相關文章: