FJCT Tech blog

富士通クラウドテクノロジーズ公式エンジニアブログです

富士通クラウドテクノロジーズ

FJCT/Tech blog

【CI戦術編 その2】コードレビューを充実したものにする方法、あるいは一生残る恥ずかしい履歴を作らないように

クラウドインフラ本部ネットワークサービス部の田上(id:rtagami)です。 ATEM Television Studio HD8が発売されましたね。 オーディオインタフェースと液タブと(以下略

前回の記事は 【CI戦術編 その1】 alembic check コマンドを活用したマイグレーションスクリプト生成忘れ防止 - FJCT Tech blog でした。

tech.fjct.fujitsu.com

連載3回目の今回は、CIでスペルチェックを実行することについて書きます。

コードレビューがtypoの指摘ばかりで本質的な議論が行えない(行われない)、 誰も気づかないままtypoの入ったコミットがマージされてしまって修正が出来ない、 ということを防ぐために、わたしたちはCIでスペルチェックを実行しています。

スペルチェックの必要性

わたしは日本語が第一言語です。 しかし、Pythonを始めとしたプログラミング言語で何かを書く際には (言語仕様としてマルチバイト文字を識別子に使用できるか、という話は置いておいて) 識別子等には英語を使うのが一般的です。 第二言語として英語を習得した(ことになっている)とはいえ、 日本語と同じレベルで英語を書けるかと言われると、 残念ながら偽と言わざるを得ません。 よって、少なくとも書いている最中にはソースコードtypoが混じることは避けられません。

しかし、typoが混ざったソースコードをコードレビューに出すことには次のような問題があります。

コードレビューが生産的なものではなくなってしまう

コードレビューでレビュアーがtypoを1つでも発見してしまうと、 レビュアーの心の中に「他にもある可能性がある」という疑念が湧きます。 レビュアーはtypoの発見・指摘に全力を注ぐことになり、 他のコードレビューの観点に対する注意がどうしても疎かになってしまいます。

※コードレビューの観点は google-eng-practices-ja などを参照されると良いでしょう。

見逃されると一生の負債(あるいは恥)を抱えることになる

コードレビューでレビュアーがtypoを見逃してしまうと、 typoの混ざったコードがプロダクションコードにマージされます。 あとからリファクタリングできる(しやすい)場所にtypoがある分にはまだマシですが、 OAS(OpenAPI定義)やDBのテーブル・カラム名などにtypoが入ってしまうと、 そう簡単には変更できず一生恥ずかしい思いをする事になります。


このような問題を防ぐためには、ソースコードをスペルチェックし typoを修正してからコードレビューに出すことが望まれます。

わたしたちはルールを作る際には必ずCIで強制することにしています (CIで強制できないルールは作らないことにしています)ので、 スペルチェックもCIで強制することにしました。

スペルチェックのCIへの導入

ようはソースコードの中にtypoがある場合はnon-zeroでexitし、 typoがない場合はzeroでexitするスペルチェッカーを用意できればCIに導入できます。

そのようなスペルチェッカーの実装をいくつか経て、今は バランスよく感度・特異度が共に高いcSpellを使用しています。

ここでは、既にtypoが入ってしまっているリポジトリに対してスペルチェックを導入するものとして書いていきます。

コンテナイメージの作成

残念ながらcSpellの公式コンテナイメージが存在しないため、 お手製コンテナイメージを作成しておきます (わたしたちのCIのジョブはすべて何かしらのコンテナイメージの上で実行されます)。 あわせてPythonの辞書も追加しておきます。

Dockerfile の例:

FROM node:lts-alpine

RUN npm install -g cspell @cspell/dict-python && cspell link add @cspell/dict-python

設定ファイルの準備

設定ファイルを作成します。わたしたちは Create a configuration file. をほぼそのまま使っています。

除外ファイルの指定・独自辞書の作成とIn Document Settingsの適用

除外ファイルの指定と独自辞書を作成します。

基本的にはプロダクションコードは全てスペルチェックされる事が望ましいですが、 わたしたちは(前回もご紹介した)Alembicのマイグレーションスクリプトだけはスペルチェックから除外することにしました。 マイグレーションスクリプトはプロダクションコードから従属的に生成される性質があり、 typoを含むプロダクションコードから生成されたマイグレーションスクリプトを あえて修正するほどの価値を感じなかったためです。

cSpellが不正な単語と認識した場合は、以下の3つの対応を適切に使い分けて対処します。

独自辞書には(ドメイン駆動開発の文脈での)ユビキタス言語やライブラリ名などの、 固有名詞相当のものを追加しました。現時点で150語ほどが登録されています。 独自辞書を濫用するとスペルチェックの感度が落ちるので、 リファクタリングの手間と天びんにかけながら慎重に検討する必要があります。

リファクタリングが難しい場所にtypoがあるが、 これからは正しいスペリングを使っていきたい(今後はスペルチェックで弾かれるようにしたい) 単語はIn Document Settingsで無視させるのが適切でしょう。 そのような単語は腐敗防止層にのみ存在し、 腐敗防止層より内側はリファクタリングで修正できることが望まれます。

CIでのcSpellの実行

ここまで準備ができれば、あとはCIで実行するだけです。 前述の手順で作成したコンテナ上で、以下のようにcSpellが実行されるようCIを設定しておきます。

cspell --no-progress "src/**/*.py"

cSpellは1つでもtypoがある場合はnon-zeroでexitしますので、 一般的なCIの仕組みと同様に、これを条件としてCIを失敗させています。

わたしたちは、明確な依頼がある場合を除いてCIが成功していることをコードレビューに着手する条件としていますので、 (CIがパスしている以上)typoはまず無いだろうという気持ちの元コードレビューに臨めます。

さいごに

今回はcSpellによるスペルチェックについてご紹介させていただきました。

cSpellは、スペルチェッカーなので当然ですが、 スペルは合っているものの単語の選択が不適切だったり、 スペルは合っている単語同士を組み合わせた文の文法に問題があるなどの問題を見つけてはくれません。 ですのでコードレビューでの命名の確認などは引き続き注意深く行っていく必要があります。

ですが、CIへの導入は比較的簡単ですし、 最小限の辞書のメンテナンスで効果的にtypoを防止できますので、 エンジニアリング上のコストパフォーマンスはかなり良いと考えています。

コードレビューが頻繁に紛糾するのでどうにかしたい、あるいは、 より効果的なコードレビューが出来る環境を作りたいという方の参考になれば幸いです。

次回は 【CI戦術編 その3】Pythonのimportのことならまかせろ isort - FJCT Tech blog になります、是非お楽しみに。

tech.fjct.fujitsu.com