自動化 Pull Request 的那些日子

Published in

AsiaYo Engineering

13 min readDec 21, 2018

你也許曾經是個 PR reviewer？你有沒有遇過 coding style 不符合標準的 committer？一上環境 linter 一跑就炸裂給你看。又或者你曾經遇過 committer 的 PR 描述奔放不已，沒有前因後果讓你不知如何審閱。本文將探討 PR reviewer 與 committer 之間的愛恨情仇與糾葛，以及我們如何讓這世界充滿祥和之氣。

在上一篇用 Drone 打造 CI/CD flow 的文章裡我們談到了當程式碼進入到 git server 上之後的自動化流程，但在進入 git server 之前的世界又是怎麼樣的呢？

AsiaYo 的開發流程

在 AsiaYo 的開發流程依照專案不同會採用 github flow 或者 git flow，一般開發者會從公用的 repository 上 fork 出一份自有的 repository，先在自己的 repository 上進行開發完畢後，送出 PR 到公用 repository 交由 reviewer 審閱，確認無誤後再 merge 到對應的 branch。

Reviewer 與 committer 的苦

由於我們的 CI flow 會進行 linter 與 unit test 的檢查，時常會發生 merge 進 branch 後結果因為檢查不過而 build failed 的情況，然後 reviewer 又需要重新再看一次 PR，一來一往耗費掉不少時間。為了解決這個問題，我們希望讓開發者在 commit 的時候就能先跑過一次 linter 與 unit test，讓 reviewer 能更專心在程式細節上

另外在前一篇文章中提到當 production released 時我們會自動產生一份 change log 來描述不同版本之間的修改，這份 change log 其實是利用一套叫 commitizen 的 cli library 來達成，而為了產生這份 change log，開發者也需要依照 Conventional Commits 的規範來撰寫 commit message。

但若每次都需要 committer 手動下指令跑 linter 與 unit test，然後再執行指令來撰寫 commit message，這整個流程對 committer 又太過不友善，身為一個 RD 是不允許自己做太多人工的事情的！因此我們想了一個架構來解決這件事情。

Githooks 的美妙之處

我們的想法其實很簡單， git 本身有提供 hooks 的功能，可以讓你在各種 git 流程節點的前或後進行 injection，我們使用到的 hooks 有下列三種

pre-commit
pre-rebase
post-rewrite

pre-commit 與 pre-rebase 都淺顯易懂不多做說明，post-rewrite 則是發生在會 rewrite git commit 的動作之後，如 git commit --amend 以及 git rebase 。底下分別貼出這三個 hooks 的程式碼並逐一說明，或者也可到bitbucket repo觀看

pre-commit

#!/usr/bin/env bash
function xdebug_disable() {
    if [ -f /etc/php7/conf.d/xdebug.ini ]; then
        mv /etc/php7/conf.d/xdebug.ini /etc/php7/conf.d/xdebug.ini.bak
    fi
}function xdebug_enable() {
    if [ -f /etc/php7/conf.d/xdebug.ini.bak ]; then
        mv /etc/php7/conf.d/xdebug.ini.bak /etc/php7/conf.d/xdebug.ini
    fi
}function error_cleanup() {
    xdebug_enable
    exit 1
}PROJECT=$(git rev-parse --show-toplevel)
STAGED_FILES_CMD=$(git diff --cached --name-only --diff-filter=ACMR HEAD | grep \\.php)
 
# Determine if a file list is passed
if [ "$#" -ne 0 ]
then
    oIFS=$IFS
    IFS='
    '
    STAGE_FILES=$(echo $@ | tr " " "\n" | grep \\.php)
    IFS=$oIFS
fi
STAGE_FILES=${STAGE_FILES:-$STAGED_FILES_CMD}if [ "$STAGE_FILES" == "" ]
then
    echo "No modified php files. Skip it."
    exit 0
fiecho "Checking PHP Lint..."
xdebug_disable
php-cs-fixer fix --using-cache no --config .php_cs.dist $STAGE_FILES --dry-run
PHP_CS_FIXER_ERROR=$?
if [ $PHP_CS_FIXER_ERROR != 0 ]
then
    echo -e "\nKerkerker... Auto fix the error but you should check first."
    php-cs-fixer fix --using-cache no --config .php_cs.dist $STAGE_FILES > /dev/null 2>&1
    error_cleanup
fiecho "Checking PHP Syntax..."
for FILE in $STAGE_FILES
do
    php -l -d display_errors=0 $PROJECT/$FILE
    PHP_SYNTAX_ERROR=$?
    if [ $PHP_SYNTAX_ERROR != 0 ]
    then
        echo "Fix the error before commit."
        error_cleanup
    fi
    FILES="$FILES $PROJECT/$FILE"
doneif [ "$FILES" != "" ]
then
    echo "Running Code Sniffer..."# Ignore database related files from goddamn laravel dirty style
    ESCAPE_FILES_REGEXP='.*database/.*/.*\.php'
    TMP_DIR=/tmp/$(date +%s)
    mkdir -p $TMP_DIR
    for FILE in $STAGE_FILES
    do
        MATCH=$(echo $FILE | grep -Eo $ESCAPE_FILES_REGEXP)
        if [ "$MATCH" == "" ]
        then
            mkdir -p $TMP_DIR/$(dirname $FILE)
            git show :$FILE > $TMP_DIR/$FILE
        fi
    done
    phpcs --standard=PSR2 --encoding=utf-8 -p $TMP_DIR
    PHPCS_ERROR=$?
    rm -rf $TMP_DIR
    if [ $PHPCS_ERROR != 0 ]
    then
        echo "Fix the error before commit."
        error_cleanup
    fiecho "Find Unit Test executable file..."
    PHP_UNIT_EXEC=$(find $PROJECT -maxdepth 4 -size -1k -regex '.*/vendor/bin/phpunit$')
    PHP_UNIT_XML=$(find $PROJECT -maxdepth 2 -name 'phpunit.xml')
    if [ "$PHP_UNIT_EXEC" == "" ] || [ "$PHP_UNIT_XML" == "" ]
    then
        echo "Not found. Skip unit test."
        exit 0;
    fiecho "Running Unit Test..."
    $PHP_UNIT_EXEC --configuration $PHP_UNIT_XML
    UNIT_TEST_ERROR=$?
    if [ $UNIT_TEST_ERROR != 0 ]
    then
        echo "Fix unit test error before commit."
        error_cleanup
    fi
fi
 
exit $?

這份 pre-commit 是針對 php 程式所做的特化版本，因此用了下面兩個指令撈出 stage area 中 .php 結尾的檔案。

PROJECT=$(git rev-parse --show-toplevel)
STAGED_FILES_CMD=$(git diff --cached --name-only --diff-filter=ACMR HEAD | grep \\.php)

接著就是運用 php-cs-fixer 與 phpunit 去跑 linter 與 unit test，若有錯誤則會終止此次 commit，並嘗試修正 linter 的 syntax error。

pre-rebase

#!/usr/bin/env bashecho $1 > .git/rebase-upstream-commit

這段 pre-rebase 雖然只有一行，但是能促成下面 post-rewrite 成功的關鍵一行，下面再進行說明。

post-rewrite

#!/usr/bin/env bash
case "$1" in
  rebase)
    TMP_UPSTREAM_FILE=".git/rebase-upstream-commit"    UPSTREAM_COMMIT=$(cat $TMP_UPSTREAM_FILE)
    UPSTREAM_COMMIT_SHA=$(git merge-base $UPSTREAM_COMMIT HEAD)
    DIFF_FILES=$(git diff --name-only --diff-filter=ACMR $UPSTREAM_COMMIT_SHA HEAD)    rm -rf $TMP_UPSTREAM_FILE
    exec .git/hooks/pre-commit $DIFF_FILES
    ;;
esac

上面有提到會觸發 post-rewrite 的 command 有 git commit --amend 以及 git rebase ，前者會被 pre-commit hook 捕捉到，所以不需在此 hooks 特別處理檢查的邏輯，後者才是我們需要特別處理的對象。

當初在處理 rebase 時遇到的問題是 HEAD node 與 rebase node 的 code base 不一定相同，該如何知道 HEAD node 與 rebase node 中間的差異？在 post-rewrite hook 中我們無從得知 rebase node 是哪一點，因此我們利用了 pre-rebase 取得 rebase node 的 commit sha，將其放入一個暫存檔案，然後等 hooks 執行到 post-rewrite 之後，再比較 stage area 與 rebase node 的差異檔案，然後執行一次原先 pre-commit 的流程。如此一來我們就可以成功讓 rebase 流程自動執行 linter 與 unit test 的檢查。

完成這三個 git hooks scripts 之後，將其置入專案資料夾，從此我們就可以在各種 git commit 的情境中自動跑 linter 與 unit test，減少了 reviewer 與 committer 的困擾。

自動規範 commit message 與 docker 好棒棒

有了上述的 hooks 解決了 linter 與 unit test 的問題之後，下一個我們面對的是如何讓開發者懶惰(x)方便(o)的撰寫 commit。最簡單的方式是要求大家的開發環境安裝 Commitizen，但因為安裝 Commitizen 需要安裝 node 與 npm(or yarn)，基於讓團隊成員可以更懶惰(x)方便(o)使用的初衷，我們打包了一個 tester tool 的 docker image，將 Commitizen 連同上述 linter 與 unit test 需要的工具一併包入，再透過撰寫一個簡單的 git command 來 trigger docker container 做事，從此天下太平，開發者只需要安裝 docker 與 git command 即可完成自動化規範 commit message, coding style, unit test 檢查。

詳細的 docker image 內容可以到 docker hub 的 graffine/php-tester repository 觀看。底下列出自行撰寫的 git command 內容

git-ay

#!/bin/sh
function cleanup() {
    echo "Clean up docker container..."
    docker container prune --force
    exit 0
}GIT_ROOT_DIR=$(git rev-parse --show-toplevel)
docker run -it -w /var/www -v $GIT_ROOT_DIR:/var/www -v $HOME/.gitconfig:/root/.gitconfig graffine/php-tester sh -c "git cz"cleanup

此 command 十分簡單，執行 git ay 之後，會 trigger 一個 docker container 起來並執行 git cz 指令，然後就一路跑 linter 與 unit test 了，產生的效果如下圖：

Reviewer 的第二個苦

做完上述動作之後解決掉了大部分 PR review 過程中發生的問題，但還有一個很致命的問題沒有解決，就是有時 reviewer 會不知道到底要 review 什麼 (爆)。一般常見的場景會是 reviewer 收到一份幾百或上千行的 PR，然後沒有來龍去脈以至於不知從何看起，為了解決這個問題我們用了一套 Refined bitbucket 工具來處理。只要在你的 git root folder 裡放入一個 PULL_REQUEST_TEMPLATE.md，在發起 PR 時他會自動將此 template 的檔案填入 PR decription 裡，效果如下圖

至此，自動化過程總算是大功告成了。

小結

這套流程開發完成後的成就感蠻大的，有了這一整套自動化架構之後，在不同語言的專案只需要選擇好 linter/unit test tool，即可快速套用同一套工作流程，讓開發環境一致化。

在環境建置上也儘可能減少開發環境需要安裝的工具，降低上手的速度，從而讓開發產出能有一個最基本的水準在。

在這套架構裡結合了許多蠻基本的工具，諸如 docker, git hooks, shell script 等等，筆者本身是 VIM 的重度使用者，也透過這次建置使用了其他不同的 git hooks (post-checkout/post-commit/post-merge) 在程式碼環境變動時自動做到類似 IDE 的 text searching 功能，這部份也許日後有機會也可以分享出來。若有其他問題，也歡迎與我討論。