慘，給Go提的代碼被批麻了
上篇我找到了一個Go的Benchmark執(zhí)行會超時的Bug，之后我就向Go提交了一個PR進行修復，本想等著代碼被Merge進去，以后也可以吹牛說自己是個Go的Contributor，但事情并不順利，今天就來分享一下這次失敗的代碼提交。
第一次提交
在我意識到Bug時，就迫不及待想去修復，于是有了這一次提交。
在說代碼前，先說點關(guān)于Go倉庫的問題，Go并沒有直接托管在github，而是自建的Gerrit Code Review，github上只是個鏡像倉庫，所有在github上提交的issue和代碼都會被一個機器人搬運到Gerrit上。
而且Go對提交代碼的要求是必須關(guān)聯(lián)一個issue，于是我就提了一個，自問自答了屬于是。
描述了一下遇到的問題，但隔天被一位大佬認為是重復問題，并且關(guān)閉了這個issue
但我點進去仔細看了下，和我說的應(yīng)該沒有關(guān)系，他們討論的是單測超時不生效的問題，于是我狡辯了一下。
果然狡辯是有用的，另一位大佬同意我的觀點，于是我給他點了個贊，但他也指出我的代碼存在問題。
下面進入今天的正題，為了便于講解，我先把有問題的代碼段摘出來：
func?(b?*B)?launch()?{
???...
????//?n（int64）可能會溢出
???n?=?goalns?*?prevIters?/?prevns
???...
}
既然知道n會溢出，還不簡單？加個判斷就完了。
溢出考慮不全
這位大佬說我的代碼在防止int64溢出時不夠安全，難道溢出不是這樣判斷嗎？
不過還好，大佬給了一點點指導
同時也發(fā)來一段演示代碼
果然 「show me the code」 最好使，簡單點來說就是正數(shù)溢出成了負數(shù)，再溢出就又是正數(shù)，只要溢出足夠多，結(jié)果可正可負。
得有測試
大佬還指出了另一個問題，兄弟，你寫的代碼得有有測試??！
雖然我給開源項目提交代碼不多，但也知道這點，為什么這次沒寫呢？主要是我覺得單測不太好寫，既然大佬提出來，硬著頭皮也得寫了。
第二次提交
第二次提交，改掉了之前判斷int64溢出的方法，用逆運算還原回去和原值做對比來看是否溢出，這個方法上次用到還是在大學的C語言課程中
還附加了一個單元測試
這個單元測試稍微解釋下：
設(shè)置了150s的單測時間，每次試探單測時，次數(shù)都加1，如果試探次數(shù)超過6次，就說明有問題，終止單測。
這段代碼在上述溢出判斷加之前執(zhí)行，一定是失敗的，溢出判斷加了之后，則可正常執(zhí)行。
接下來就是等待回復，等了很久很久，Go的研發(fā)周期是以半年記，等得我都差點忘了這件事了，直到一天郵件提醒我。
前方高能，來看看另一位大佬是如何review我的代碼的。
commit message不規(guī)范
首先，commit message不規(guī)范，我的commit在github上提交的，被機器人搬運過去。
給出的意見是
原來Go的commit message有一個文檔專門介紹的，之前沒注意到，點進去看了下
翻譯下就是commit message的第一行應(yīng)該是簡短的摘要，并且要指出影響了哪些包，第一行后得有一個空行。
commit message的主要內(nèi)容應(yīng)該詳細說明變更的上下文，并解釋其作用，語句完整、標點正確，不要使用HTML、Markdown等標記語言。相關(guān)的信息，如基準測試數(shù)據(jù)等也需要寫進來。
最后需要有關(guān)聯(lián)的issue，如果是修復某問題，需要用Fixes #12345來關(guān)聯(lián)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。
可以考慮集成測試
單測提了不少問題，首先是這個
我把Benchmark的單測包名改了，改這個是為了能調(diào)用包內(nèi)未導出的方法，確實不太好，但當時沒想到別的方案。
接著是不應(yīng)該直接調(diào)用未暴露的cleanups和內(nèi)部的一些變量，和上面呼應(yīng)。
可以用flag.Lookup來set flag，這點沒用過，所以不知道。
或者可以考慮使用集成測試來代替單元測試，Go的集成測試在cmd/go/testdata/script，這個之前也沒接觸過，所以也不知道，這個集成測試具體怎么用可以看cmd/go/testdata/script/README
這點可以看出我真是個Go新手，需要多看多學，測試不光只有單測，Go還支持集成測試。
缺少注釋
再接著看
這里模擬150s的單測，大佬就提問了，這個單測真的會跑150s嗎？如果是的話，那也太長了！
如果不是，也沒給我解釋清楚啊~
還有這個
你咋知道執(zhí)行次數(shù)一定小于6呢？Go可沒保證這個。
對于這兩點的疑問，核心問題在于沒寫注釋，別人不知道你的想法呀，如果開源的代碼里面充斥著這種看不懂的玩意，那不是要命。
首先對于第一個，模擬150s，實際上不會真的跑那么久，因為后面有試探次數(shù)的限制，如果超過6次，就終止了，這個6次是怎么得到的呢？答案其實在《我好像發(fā)現(xiàn)了一個Go的Bug》中。
Benchmark在一個方法上跑的最多的次數(shù)是1e9次，也就是1000000000次，如果待測試方法執(zhí)行時間非常短，且在Benchmark時間比較長的情況下，計算需要執(zhí)行多少次一定會溢出，所以試探的執(zhí)行次數(shù)會是這個增長序列：
100、10000、1000000、100000000、100000001、100000002......
實際可能>4就完事了，可能是我之前測試的有問題，emm...
溢出需要重新考慮
別判斷n是否溢出，如果判斷上一層，即goalns是否大于等于?int64最大值 * prevIters是否更合理呢？
n = goalns * prevIters / prevns，goalns 是設(shè)置的執(zhí)行時間（單位納秒）
看來是我格局小了，別急，還有
怎么知道100 * last是不是也溢出了呢？所以我們是不是全程的計算都用float64更合理呢？
測試了下，float64范圍大的離譜，感興趣可以試試，就不貼數(shù)據(jù)了，太長！
最后說一句
雖然這次提交比較失敗，但還是有點收獲，等我忙完這陣，抽空出來再改改，說不定就被Merge了，大家祝我好運吧，今天的分享到這，我們下期再見！對了，文中的issue參考https://github.com/golang/go/pull/50023
想要了解更多相關(guān)的內(nèi)容，歡迎掃描下方???關(guān)注?公眾號，回復關(guān)鍵詞 [實戰(zhàn)群]? ,就有機會進群和我們進行交流~
分享、在看與點贊，至少我要擁有一個叭~
