(轉載請注明出自 http://www.shnenglu.com/converse 那誰)
一直以來想找機會談談目前這個項目組內采用的代碼提交制度,今天整理一下.
分如下幾個流程:
1) 在trac系統上建立ticket,寫好這個任務的目的,并且accept這個ticket.
2) 修改代碼,把相關的修改過的代碼(一般還應包括相關的測試用例的代碼,后面會加以說明)提交到review board上.
3) 在review board上填寫如下幾個必要的信息:
a)第1)步建立的ticket的號碼(可以有好幾個,也就是一次修改可以針對好幾個任務),以明確這次提交針對的是哪個任務;
b)寫下測試了哪幾個測試用例,對這次的提交寫一段描述.
c)寫下給哪些人做review.reviewer的角色分為兩種,一種是負責人,另一種就是普通的組員.每一次提交,必須保證reviewer中有至少一個負責人,并且需要所有的reviewer都通過了這次修改,才能向代碼庫提交代碼.reviewer會針對提交的代碼進行批注回復.
4)一般情況下,代碼不會在第一次提交就能通過review,大多數情況會被打回修改,于是2)-4)三個步驟將循環進行下去直到代碼通過review為止.
5) 提交了代碼之后,1)中建立的ticket會被自動關閉,并且將在ticket的回復自動寫上本次提交修改的文件以及svn revision號,這樣以后再看起來就知道是哪次提交并且修改了哪些文件針對的又是哪個功能了.
6) 有一臺服務器專門作為buildbot機器, 這臺機器在每次提交了代碼之后,
將自動清空原來的代碼目錄,更新最新的代碼,重新編譯,然后把里面的測試用例全部跑一遍.我們的項目使用的tcmalloc,會檢查內存泄漏,所以在測試用例測試失敗,以及有內存泄漏的時候,buildbot都會失敗.需要補充一點的是,除了每次提交代碼會導致buildbot重新編譯新的代碼,在每晚的一個固定時間,即使沒有更新代碼,也會做相同的動作.buildbot的存在,就是為了不斷的清空編譯文件重新編譯再跑測試用例,以大量的測試消除隨機性保證正確性.
以上是整個代碼提交機制的大體流程說明,下面談里面的細節.
1)上面的第1)步中,建立trac的ticket時,需要指定一個milestone,一般我們對必須做的事情是每周以日期命名建立一個milestone,這樣,你做的任務就會自然的變成每周可以去跟進的任務.
2)第2)步中,寫測試用例針對的是每個類或者每個頭文件對外暴露的API接口,比如對外的API c使用內部的函數B,那么是沒有辦法對函數B編寫測試用例的.我們的要求是, 任何的一個新增的API都需要寫針對它進行測試的測試用例,不一定只有一個,因為需要考慮的情況可能很多,總而言之,盡可能的考慮齊全.假如本次修改修改了內部函數B,那么依賴于函數B的API c它的測試用例也就需要再測試了.這些測試用的代碼也會一并提交到代碼庫中,因為這樣才能保證buildbot更新之后也按照最新的測試用例進行測試.
3) 代碼提交到reviewboard之前,還需要過lint這一關,對基本的代碼風格進行檢查.我們使用的是google c++ code style.
4) reviewer中的負責人角色很重要, 起著看門人的作用,任何的一次修改提交,都必須至少經過一個負責人的review, 所以對他的要求就相對高些了,除了要完成自己的工作外,還需要認真review他人的代碼,而要review他人的代碼并且給出好的意見來,又要求他本人除了編碼能力外,還要在業務層面對別人的工作有大體的了解,不然沒法review了.
5) 從前文可以看出,review實在是一個繁雜的工作,很有可能在review階段被打回修改代碼,就我的經驗而言,提交review的人要將提交的任務盡量的劃分的細一些就來的很重要了.在提交review的時候,我一直堅持DOTDIW原則(Do One Thing,Do It Well).相反的例子,我們組有個同事,做了一個很大的功能,光是完成這個大的功能,就花費好幾周的時間,提交review的時候代碼量大,有個幾千行的,這樣別人review起來也慢,而且一旦不通過需要修改,又是一個苦力活兒.這樣一折騰,一個月時間過去了.如果當時能對整體的功能有個把握,懂得劃分模塊層次,逐個提交,也許會好些.當然,大規模的代碼提交有時并不能完全的避免,比如一個比較大的重構,牽一發而動全身的,我只是說如果可能,應盡量避免大規模代碼的提交,并且最重要的是:每次提交最好僅針對一個功能點.
以上是對流程從整體到細節的描述.現在談談我的看法.
先來談優點:
1) 通過codereview制度,保證了項目組成員之間能夠在代碼層面上直接的進行交流.我想這一點是最重要的,沒有之一.
如果你是一個水平差一些的程序員,那么有比你牛叉的人幫你review,相當于是讀書的時候有老師幫你閱卷修改作業,可以指正你的問題所在.我在被人review的過程中就聽到了別人對我很多的意見.而如果是一個水平較高的人,是不是對水平差的人進行codereview就是浪費他的時間了呢?我個人認為,這個事情分怎么看.從團隊的角度看,按照木桶理論,最短的短板往往決定了能達到的水平.總不能指望所有的事情都由老鳥完成,所以老人在幫新人review代碼的時候間接的幫助了新人的成長,同時作為項目組中資歷比較深的人,也應該對項目多費一些時間進行把關.我覺得這一點無可厚非.同時,即使是新人,通過閱讀他人的代碼并且交流,也可以學習到別人的思想.
codereview制度從上面的角度上保證了項目成員可以直接通過代碼進行交流,簡單的說,誰寫的代碼質量如何,一到了review,一目了然.寫的好的通過的快,寫的不好被打回修改多了自己也會長記性,還可以多看別人的代碼進行學習.這樣,在一定層面上可以使項目組成員的能力盡可能的接近.我能力長了一級,相應的也會拉動項目組中的人升級.
2) 測試用例.我之前提到的測試驅動開發,想法就來自于項目組中編寫測試用例+buildbot執行測試用例的做法.如何證明一個API是確定無誤的?我覺得這個問題似乎非常難.除了一些可以通過數學上邏輯上證明的情況外,還需要考慮很多其他的隨機情況.比如線程的切換是隨機的,某個文件夾恰好存在是隨機的,等等.這些隨機出現的問題,很多時候不能在某一次測試中顯現出來.但是如果有了buildbot,不停的更新,編譯,執行,總會有暴露問題的一天.另外,之前我也提到過,腦中如果有了測試用例的概念存在,每寫一個API時都會考慮到針對它的用例應該是怎樣的,從另一個角度,也幫助你的設計--你需要考慮這個API的輸入,輸出,異常情況都有哪些,如何測試到.測試用例的存在,保證項目盡量的做到了"可控制".
缺點:
1) 從上面的流程可以看出,走完一個代碼提交的流程需要花費大量的精力/時間(有一些還是他人的精力/時間),所以,也許這個制度并不適合于那種時間壓力比較大的項目.
2) 有幾個地方很難堅持,比如codereview時,有些reviewer會走過場,也沒怎么看代碼就直接通過了,這樣就會流于形式了.還有編寫測試用例,也是一件很耗時間的事情.
3) 由于codereview制度的存在,給review中的負責人帶去的壓力很大,因為本身他有自己的工作,又要盡量對他人的業務/代碼進行了解,就像那種負載很大的服務器一樣,有時候會不堪重負.這一點上,我的想法是,項目組內的其他成員,如果能積極主動一些,多花時間了解別人的代碼,盡量分擔review的任務,或者試著成長為負責人級別(降低單點故障率:),也許是一個好辦法.不過,這還得看人了.
整個代碼提交流程上的軟件,目前我所知的都是使用的開源軟件:SVN, google c++ code style lint,review board,buildbot,trac,測試用例的編寫上使用的是google的測試用例框架gtest.不過,整合起這些軟件搭建好這個流程就不是那么容易了,用了一人/半年左右的時間,才讓這個流程基本穩定下來.
總之,這套機制中,既有人為的干涉,也有機器層面上的干涉,也許并不是最完美的,但是盡了最大的力量去保證項目的正確性和可控性.
以正確的方式正確的制度做事,是將事情做好的一個關鍵.