慘，給Go提的程式碼被批麻了

捉蟲大師發表於2022-03-08

hello大家好，我是小樓。

不知道大家還記不記得我上次找到了一個Go的Benchmark執行會超時的Bug？就是這篇文章《我好像發現了一個Go的Bug？》。

之後我就向Go提交了一個PR進行修復，本想等著程式碼被Merge進去，以後也可以吹牛說自己是個Go的Contributor，但事情並不順利，今天就來分享一下這次失敗的程式碼提交。

第一次提交

在我意識到Bug時，就迫不及待想去修復，於是有了這一次提交。

在說程式碼前，先說點關於Go倉庫的問題，Go並沒有直接託管在github，而是自建的Gerrit Code Review，github上只是個映象倉庫，所有在github上提交的issue和程式碼都會被一個機器人搬運到Gerrit上。

而且Go對提交程式碼的要求是必須關聯一個issue，於是我就提了一個，自問自答了屬於是。

描述了一下遇到的問題，但隔天被一位大佬認為是重複問題，並且關閉了這個issue

但我點進去仔細看了下，和我說的應該沒有關係，他們討論的是單測超時不生效的問題，於是我狡辯了一下。

果然狡辯是有用的，另一位大佬同意我的觀點，於是我給他點了個贊，但他也指出我的程式碼存在問題。

下面進入今天的正題，為了便於講解，我先把有問題的程式碼段摘出來：

func (b *B) launch() {
   ...
    // n（int64）可能會溢位
   n = goalns * prevIters / prevns
   ...
}

既然知道n會溢位，還不簡單？加個判斷就完了。

溢位考慮不全

這位大佬說我的程式碼在防止int64溢位時不夠安全，難道溢位不是這樣判斷嗎？

不過還好，大佬給了一點點指導

同時也發來一段演示程式碼

果然「show me the code」最好使，簡單點來說就是正數溢位成了負數，再溢位就又是正數，只要溢位足夠多，結果可正可負。

得有測試

大佬還指出了另一個問題，兄弟，你寫的程式碼得有有測試啊！

雖然我給開源專案提交程式碼不多，但也知道這點，為什麼這次沒寫呢？主要是我覺得單測不太好寫，既然大佬提出來，硬著頭皮也得寫了。

第二次提交

第二次提交，改掉了之前判斷int64溢位的方法，用逆運算還原回去和原值做對比來看是否溢位，這個方法上次用到還是在大學的C語言課程中

還附加了一個單元測試

這個單元測試稍微解釋下：

設定了150s的單測時間，每次試探單測時，次數都加1，如果試探次數超過6次，就說明有問題，終止單測。

這段程式碼在上述溢位判斷加之前執行，一定是失敗的，溢位判斷加了之後，則可正常執行。

接下來就是等待回覆，等了很久很久，Go的研發週期是以半年記，等得我都差點忘了這件事了，直到一天郵件提醒我。

前方高能，來看看另一位大佬是如何review我的程式碼的。

commit message不規範

首先，commit message不規範，我的commit message是這樣的，我是在github上提交的，被機器人搬運過去。

給出的意見是

原來Go的commit message是有一個文件專門介紹的，之前沒注意到，點進去看了下

翻譯下就是commit message的第一行應該是簡短的摘要，並且要指出影響了哪些包，第一行後得有一個空行。

commit message的主要內容應該詳細說明變更的上下文，並解釋其作用，語句完整、標點正確，不要使用HTML、Markdown等標記語言。相關的資訊，如基準測試資料等也需要寫進來。

最後需要有關聯的issue，如果是修復某問題，需要用Fixes #12345來關聯12345號問題，如果只是解決部分問題，使用Updates #12345，如果修復的是golang.org/x/庫，使用Fixes golang/go#159。

一個好的例子如下：

math: improve Sin, Cos and Tan precision for very large arguments

The existing implementation has poor numerical properties for
large arguments, so use the McGillicutty algorithm to improve
accuracy above 1e10.

The algorithm is described at https://wikipedia.org/wiki/McGillicutty_Algorithm
Fixes #159

看來我得好好改下commit message。