GitHub ActionsとDanger JSを組み合わせて自動PRチェックを使い続けて気がついた事
こんにちは。
株式会社ココナラ フロントエンド開発グループの三浦です。
皆さんはこんな経験はないでしょうか？
「マージ先がdevelopになっててうっかりマージしてしまった！」
「毎回同じチェックをしてるはずなのになぜかチェックがすりぬけてしまった！」
どうです？ありますよね？
私はあります。
といった始まりのDangerを使ってみた記事を書いて早いもので11ヶ月、約１年という月日が流れてしまいました。
ちなみに上記の記事はこちらのリンクからも読むことが可能です。
「まだ読んだことないよー」という方や
「改めて読み直してみようかな」といった方は是非ご覧になってください。
Dangerとは
初めてDanger JSを知る人にむけて、一応のおさらいとしてDanger JSについて軽く説明します。
一言で言うと、チーム内で慣習的に行われているコードレビューを自動化してくれるツールです。
DangerはDangerfileと呼ばれるスクリプトを利用して、GitHubのプルリクエスト上で実行するタスクを定義します。
ココナラのフロントエンドは主にNuxt.js + Vue.jsで作られているため、JavaScript版のDangerJSを使用しています。
機能などについては前記事でも説明しているため割愛します。
Dangerで実際に何を確認しているの？
実際に運用して使っているものを一部抜粋したものにはなりますが、以下のような内容をチェックしています。
- 指定した文字列が含まれる場合に警告を出力する
- 修正された行数が300行を超えていた場合に警告を出力する
- 修正されたファイルが8ファイルを超えていた場合に警告を出力する
- マージコミットの親が2つない場合に警告を出力する
それぞれを簡単に説明します。
指定した文字列が含まれる場合に警告を出力する
このチェックが一番汎用性の高いチェックになってるかと思います。
console.をチェックするようにしておくことでconsole.logやconsole.warnなどさまざまなconsole系の消し忘れを防止できます。
これを残したままPRのレビューを出そうとした場合、おそらく自分が実装に注力しすぎて俯瞰して自分のPRを見れなくなっていることがわかるため、冷静に自分の修正を見返す良い判断材料にもなるので自分は重宝しています。
また、こういった消し忘れチェックや実装次第で注意しなければならないことを確認するときに、定義にチェックしたい文字列を追加するだけで自動でチェックをいれてくれるので良い自動チェックではないかと思います。
修正された行数が300行を超えていた場合に警告を出力する & 8ファイルを超えていた場合に警告を出力する
PRのレビューを依頼する際に気をつけるべきことがPRの大きさです。
フロントエンドグループでもかつては変更行数の大きいPRが結構レビュー依頼に飛んでいました。
しかし、現在ではそこまで大きいPRレビューはほとんどなく、多くてもこの300行という目安ができたことでPRを細かく分割して対応しようという意識が根付いたように思います。
リファクタリングでコンポーネントを分割したなどの場合、どうしても変更行数が大きくなってしまうなどの例外パターンは存在しますが意識が変わったことが体感レベルで実感できるのは純粋に良いことだなと感じています。
マージコミットの親が2つない場合に警告を出力する
自分の中では影の実力者かなと思っています。
プロジェクトが並行で走ることや突発的な修正で最新のdevelopブランチを取り込むことや、誰かの修正を自分のブランチにも取り込むためにマージを行うことが稀によくあります。
(稀によくあるという言葉の絶妙なニュアンス好きです)
その時に役に立つのがこのチェックです。
他ブランチ取り込みの際マージコミットになっていないと新たな変更と見なされ、最新の変更としてオートマージされた結果デグレにつながってしまうということがあります。
小規模なプロジェクトであればそこまで大きな問題にはなりませんが、大きめのプロジェクトでQA真っ最中などの場合目も当てられない影響が発生します。
QA対応者の障害起票、修正前の事象確認、調査、修正、再打鍵...etc...
サッと見積っただけでもこれらの修正に馬鹿にならないレベルのリソースがとられます。（実際に何度か発生したことでこのチェック項目が追加されました）
このチェックが追加されてから、チームでの意識にも「マージコミット問題なさそうか？」という意識も芽生えつつ実際に間違ったマージをしてしまった場合はこれらのチェックでレビュー前orレビュー中にデグレを防ぐことができるようになりました。
このように１年ほど運用しただけでもチーム内で「これチェックできたらいいね」といった提案ベースで色々と事故やケアレスミスなどを削減できているように感じます。
Dangerを使ってみての感想
実際に1年近く運用を続けてみて感じた事は「便利で役に立つが過信は禁物」ということでした。
上記の章で触れたように、これらの確認項目のおかげで結構な細かいリソース削減ができているんだろうなという感覚があります。
（実際に計測などしたわけではないので肌感レベルではありますが...）
ただ、これらのチェックにもやはり罠はあるなとも感じています。
それは人の慣れから発生する形骸化です。
確認不足による不具合の修正などを行っている時に痛感する内容でもありますが「チェックしてあるから大丈夫」といった慣れによる慢心から引き起こされる不具合、経験ないでしょうか？
基本的にはチェックを通っているPRが多いのですが、API繋ぎ込み前のモックでのマークアップやTODOを残した上で細かいPRでレビュー依頼を投げる場合、動作確認用にチェックに引っかかるRPを出すこともあります。
そうしているうちに「これ毎回言われるんだよなぁ」という気持ちになりついwarn出力をスルーしてしまう、心当たりありませんか？
私はあります。大有りです。
また、何か不具合が発生した場合それらを回避する案として上がりやすいというのも気をつけなければならない点です。
便利だからという理由でなんでもwarnを出しすぎると本来防ぎたかった事象が埋もれてしまい、完全に意味をなさないチェックになるリスクが伴います。
なので、便利で役に立つが過信は禁物という感想に着地しました。
最後に
ココナラではエンジニアを募集しています。
よろしければぜひ以下のページもご覧ください。
フロントエンドの求人はこちらです。
Discussion