2018
Apr
16




Code Review 是一件很重要的事,很多 Bug 都可以透過 Code Review 來解決,學會怎麼 Review 別人的程式更是一門學問,除了 review 程式邏輯外,其它還有很多地方要特別留意。

  • Beta / Production 環境:一般軟體開發都會有區分多個環境,最少有 Beta(staging) , production 兩個環境 ,工程師很容易只針對其中一個環境來開發測試, Commit 的程式也只測過 Beta,並沒有想到 production 的部分 ,所以 review 時要看看新增的程式是否有處理不同環境的設定,最常見的就是加了一個 Beta 測試用的設定值,而少加了 production 所需的設定。
  • 是否有 Require library, Import lib path,有些工程師寫程式都不測試的,程式邏輯寫完了,卻沒有載入對應的 library ,一執行就直接掛點,建議要求所有 RD 寫 unit test or Integration test 來避免這種問題,若是公司內沒有這些規定,那麼 review 時就多看兩眼。
  • Package dependency:如果有升級 Library / Package 要記得改版號,工程師很常自已的環境測試完,就忘了改 pom.xml / package.json 等版號。
  • Reuse function: 想想看是否有其它工程師曾經實作過相同的程式邏輯,盡量 Reuse 同一段程式,不要重覆寫一模一樣的 code。
  • 極端值: 程式是否能夠處理各式各樣奇怪的數值,例如負數,0 ,big number ,null,空字串,undefined,large array。
  • 可讀性:保持程式簡單易懂,如果程式邏輯著實複雜得誇張,可以把複雜而不常更動的部分打包藏進 Library,讓外層程式 flow 乾淨好懂 (Facade pattern)。
  • 程式是否會影響到其它的 service ,input / output 改變或是底層 library 變更,有可能會產生其它服務 Bugs 。
  • 程式一致性:不要創造新的變數名稱,保持原來程式的命名,但是如果原程式命名有很大的問題,那就把有問題的變數全部改掉,不要只改你有動到的地方。
  • 把自已當成一個測試工程師,測試一下程式邏輯是否有 Bug。
  • 是否有機會提升效能。
  • 是否有安全性漏洞,例如 SQL Injection, XSS, File Injection, Buffer overflow。
  • 前置作業: 新程式是否需要搭配修改 DB Schema,或是必需先修改 production settings,是否需要多增加一個開關設定上線日,時間到才自動啟用新功能,如果沒有事先設定好這些,程式 release 當天很可能就立馬倒站,以下幾種狀況都會造成系統出錯。
    • 修改 Code SQL 語法,但是 DB Schema 沒有改。
    • 程式需搭配 DB table index,如果沒有先建好 index 會造成 Slow Query 。
    • Production 需增加新的設定,而設定與程式邏輯分別在不同的 Repositories,甚至是安裝在不同的機器,無法同時 Release 。
  • Release:程式是否能夠 release ,有些功能還在開發當中,不能公開給外部的 User 使用,如果 merge 這段程式,是否會造成提前 release 。
  • Rollback 機制:如果新版程式在線上運作一段時間後,出現問題要如何 rollback。
  • Release 流程:一般線上機器不會只有一台,一般來說至少有 3 台以上,在 Release 的過程中,會一台一台慢慢安裝,造成線上有一部分機器是舊的程式,一部分機器已經安裝新的程式,新機器建立的資料格式,舊版的程式無法正確處理,而造成系統 crash。
  • 資料格式改變:移除即有欄位,很可能會造成系統錯誤。
    • Cache 內容變更:memcached, redis, http cache, browser cache, session cache 以上 cache 都會存在系統一段時間,資料格式改變時,並不會把即有的 cache 一並更新,這時線上會同時存在新舊兩種 cache,需確定程式讀取 cache 時,不會因格式改變而出錯,另外在測試的過程中就有可能因為新版的 cache 資料造成線上環境出錯,很可能在 review 的當下就已經有新版的 cache 格式出現在線上環境。
    • 回傳格式改變,前端是否有相對應的變更,Release 流程是否要調整。
    • 是否需要 On the fly data migration
  • Merge 順序:如果同時有多個 Pull Request,每個 PR 相依性是否有正確設定,底層 Library 優先 Merge 。

回應 (Leave a comment)