一次優(yōu)秀的代碼提交應(yīng)該包含什么?
先讓我們來聽一個令人惡心的例子。
你看到 問題 F00-123
被解決了。這是關(guān)于一個你自己很 熟悉的子系統(tǒng)的 Bug,所以直覺告訴你造成這個 Bug 最可能的原因。為了證實你的懷疑,你決定看看這個 bug 是怎么被解決的。你花了很長時間搜索了整個版本歷史,直到把這個 bugfix 的范圍縮小到了 4 個連續(xù)的提交,它們分別的提交信息是: dao 小調(diào)整(dao tweaks)
、 moar
、 Fixes
, 還有 刪除一些調(diào)試信息(remove debug stuff)
。每個提交的修改集看起來都很大,多達十幾個文件的幾百處修改。“我艸尼@#$%%^&”,你準(zhǔn)備罵娘但還是停住了,沒有罵出你腦中那句最難聽的粗口。“這個 bugfix 不應(yīng)該超過三行代碼!”。
上面的例子聽起來是不是很熟悉咧?很多開發(fā)者僅僅只把版本控制系統(tǒng)當(dāng)成是一堆文件的備份。這些歷史備份除了可以讓你取回某個時間點上的文件內(nèi)容之外,沒有任何其他用處。下面這些建議可以幫你把你的版本控制系統(tǒng)從一個備份系統(tǒng),變成用于溝通和文檔編寫的一個寶貴的工具。
1. 每次提交只做一個修改
如果你在一次提交中修復(fù)了 F00-123
和 F00-233
、重構(gòu)了一個類、界面上添加了一兩個按鈕、還有把整個項目文件中的 tab 改成空格,那么基本上沒有人能 review F00-123
的 bugfix,只有你自己才知道哪些修改屬于那個 bugfix 的一部分,但是一周以后很可能你自己都忘了。
要是一周以后你發(fā)現(xiàn)原來的那個 bugfix 導(dǎo)致了另外一個更嚴(yán)重的問題那你就完蛋了,你不能使用 hg backout
或者 git revert
來撤銷你的修改,因為那樣會把除了那個 bugfix 之外的所有修改都撤銷掉,也意味著你一周的努力白費了。
上面這種問題的解決辦法就是“每次提交只做一個修改”,對于怎么樣才算是“一個修改”沒有任何硬性或者既定的規(guī)則,但如果你能夠用一個句子并且不用用到“和”這個字來描述你所做的所有事情,那么你算是做到了。
分布式版本控制系統(tǒng)其中一個很好的功能就是,如果你的工作目錄中有很多相互之間沒有關(guān)聯(lián)的修改,你可以用它來幫你 收拾好你的爛攤子(clean up the mess you’ve made),但是***別在一開始就搞出一個爛攤子。在你開始修改代碼之前,先確定你想干什么和你想怎么做,然后再認(rèn)真把關(guān)注點放在你想修改的那個點上。
在沒有想出怎么改進某塊代碼之前,貌似不太可能去修改那塊代碼。你發(fā)現(xiàn)了 bug、看到了一些糟糕的代碼、還有一些你很好奇想進一步了解的東西。無論你多么想去做,千萬別被轉(zhuǎn)移了注意力。這些發(fā)現(xiàn)非常有價值,用一個筆記本或者一個 TODO 文件把他們 寫下來(jot them down),在完成當(dāng)前任務(wù)之前不要想去解決那些問題。
這個不僅僅是關(guān)于更好地提交。當(dāng)你沉浸在解決某個編程問題里面的時候,你的頭腦充滿著關(guān)于這個問題的一堆細(xì)節(jié),如果這個時候你跳去想一些其他問題,那么你會忘了原來那個問題的細(xì)節(jié),再回到原來那個問題需要花一些時間。你需要 減少任務(wù)切換(minimize task switches)來提高你的工作效率。
當(dāng)然有些時候你會發(fā)現(xiàn)在沒有解決某些其他問題之前,你很難完成當(dāng)前的任務(wù)。這個時候最簡單的方式就是使用 hg shelve
或者 git stash
把你當(dāng)前還沒有完成的修改暫存起來,把兩個問題的修改分開,提交那個被依賴的修改,再回到你原來的任務(wù)上面去。
2. 每次提交要包含完整修改
如果一個修改分布到了幾個提交里面,那么這個修改也是很難 review 的。通常這是因為在同一時間解決太多問題導(dǎo)致的,如果你吃不了那么多,卻啃了那么多,那么在你想保存其中的某些修改的時候,你會發(fā)現(xiàn)大部分的修改都是未完 成的。同一時間關(guān)注太多問題會導(dǎo)致***需要很長時間才能夠提交完整的修改。(原文是:Focusing on one task at a time takes you a long way towards committing complete changes. 但我覺得作者的原意應(yīng)該是:Focusing on too many tasks at a time takes you a long way towards committing complete changes.)
有些修改需要花很長時間,所以如果中間在某個點上你搞 錯了,重頭再來你肯定折騰不起,所以你需要在過程中保存一些中間版本。幸好分布式版本控制系統(tǒng)都允許你保存很多中間版本但是***只提交一次修改到中心版本 庫(central repository),你可以提交任意多次的中間版本,在***使用 hg histedit
或者 git rebase
把多次的中間版本合并成一個修改集。
另外一種,也是我個人比較喜歡的方法,因為它把中間版本和***性的修改版本分開了,這種方法使用 Git 的 index,或者 Mercurial Queues 中的一個 patch 來保存最近一次正確的(沒有 bug 的)中間版本,每次你做了新的修改你都更新這個 index/patch,如果你犯了個錯誤,你就可以使用他們恢復(fù)你的工作目錄了。I like to think of it as a one-slot quicksave for version control.(譯注:這句只可意會,我不知道怎么翻。)
3. 寫好注釋,說明你修改了什么
像“修復(fù)(Fixes)”、“提交(Commit)”這樣的提交信息沒包含任何有用的信息。如果別人想看看版本歷史,像這樣的提交信息只會逼他們?nèi)タ赐?所有的代碼修改,看代碼是很費時費力的。寫這樣一個簡短但表達不清晰的提交信息,可能是省了你一分鐘時間,但是卻浪費了其他人幾個小時。
一個好的提交信息可以讓看的人清楚代碼的哪一部分被修改了,是怎么被修改的,他們也不需要去看你的代碼:
- SomeClass: use bleh instead of xyzzy in someMethod (fixes FOO-123)
4. 注釋說明為什么做這個修改
假設(shè)每次修改代碼都有一個很好的理由/原因,但如果這個理由/原因被沒有記錄下來,那么整個代碼庫(codebase)將面臨以下風(fēng)險:
- 其他開發(fā)者不明白為什么原代碼是那樣寫的。當(dāng)他們修改代碼的時候,他們可能會引入一些原作者已經(jīng)發(fā)現(xiàn)或者避免的問題。
- 其他開發(fā)者認(rèn)為原代碼那樣寫肯定是有(好的)原因的,所以***別動它。他們把這些代碼看成是一個黑盒,然后加各種復(fù)雜的 workaround,避免修改原代碼。***導(dǎo)致這個代碼庫變得臃腫,代碼變得難以看懂。
如果你有足夠的理由有必要破壞一個項目的規(guī)范或者約定,那一定要把這個理由作為注釋寫在你的代碼里面:
- - xyzzy (bars); + // Our bars are already sorted, so bleh is much faster than xyzzy + bleh (bars);
如果你的代碼遵守了規(guī)范,并且你的代碼沒有什么微妙(subtleties)的點需要注意,那就沒有必要把你的文檔注釋寫在代碼里面。但仍然有必要讓人知道為什么新代碼優(yōu)于舊代碼(尤其是當(dāng)新代碼引入了一個新問題),所以還是要把原因?qū)懺谔峤恍畔⒗锩娴模?/p>
- SomeClass: Don't flush caches in someMethod The caches are flushed automatically at the end of each request.
如果一個修改解決了一個已知問題,確保在提交信息里面帶上 ticket 號(bug 追蹤系統(tǒng)中的 ticket),以便其他開發(fā)者在看版本歷史的時候能夠清楚地知道是在什么情況下做出的這個修改。
5. 不要提交被注釋掉的代碼
我沒有辦法理解提交被注釋掉的代碼背后的理論依據(jù),我假設(shè)這是為了保存舊代碼,以防新代碼不能正常工作,但這種做法很莫名其妙,最開始我們使用版本控制系統(tǒng)不就是為了保存舊版本嗎?!
為什么要注釋掉這些代碼?這些代碼能運行嗎?會正常運行嗎?曾正常運行過嗎?注釋代碼是我們應(yīng)該支持還是摒棄的呢?被注釋掉的代碼毫無用處,因為每當(dāng)開 發(fā)者讀到這些被注釋的代碼,總會冒出一些沒有答案的問題,它只會混淆開發(fā)者視聽,讓開發(fā)者分心而無法更好專注于有用的代碼。
對于提交被注釋掉的代碼這件事情,只有一個原則,那就是:
原文鏈接:http://dev.solita.fi/2013/07/04/whats-in-a-good-commit.html
譯文鏈接:http://kb.cnblogs.com/page/181762/http://kb.cnblogs.com/page/181762/