コードレビューについての覚書

最近、多少まとまった時間を使ってコードレビューをする機会があったので、僕の考えるコードレビューのポイントを備忘としてまとめておく。

ただし、以下は前提。
* Github、あるいはそれに類似したレビューシステムを利用している。SCMはGit。
* 商用というか、仕事のコードレビューである。

---

プルリクの目的が明確であり、レビュワーがそれを理解できるようになっている

そのプルリクが何をしようとしているのか、レビュワーはレビュー前に理解していないといけない。だからプルリクの最初のコメントでそれがきちんと説明されているかを確認する。目的がわかればレビューにどれだけ力をかければ良いかもわかる。例えば一回だけしか使わない運用スクリプトなら、細かいフォーマットの崩れとかを逐一指摘する必要はないが、実行内容の正しさは十分注意してみないといけない。

差分を見ればそのプルリクで何をやっているかわかる、という主張ももしかしたらあるかもしれないが、それはちょっとやろうとしていることが逆転している。目的に対してコードの修正が正しいことを確認しなければいけないのに、書かれたことからやろうとしていることを忖度しようとしたら、本来の目的でないものをOKとしてしまうかもしれない。

Githubのプルリク画面だけではなく、checkoutして全体のコードを見て、修正漏れがないことを確認する

当たり前といえば当たり前だが、ある程度の修正をちゃんとレビューするならプルリクの画面だけでは仕切れない。一旦ローカルにチェックアウトして全体をレビューする必要がある。変更する必要があるのに漏れているケースなどは差分には出てこない。修正漏れがないことを確認する必要がある。チェックアウトした上でのgrepはとても効果がある。うまくキーワードを見つけてgrepする。特にコードを削除する修正の時など、削除もれがないように注意する。削除すべきタイミングでしないと、消して良いかわからないコードが残り、これを消すのに勇気が必要になってしまう。（本来自動テストで担保するべきではあるけど）

プルリクの目的に対して、変更点が適切である

機能を追加しようという時に、追加するべきファイルが適切かを見ることで、大枠の設計として問題ないかを確認する。git diffの--name-onlyオプションが便利。ちなみに最近覚えたけど、リモートネームを指定すればローカルにマージ先をチェックアウトする必要はない。例えばmasterにfeature_Aブランチからプルリクが出ている場合は、以下のコマンドで修正ファイルを確認する。

$ git checkout feature_A
$ git diff origin/master —name-only

プルリクの目的外の問題を見つけたら、（別の問題であっても）指摘する

これは異論があるかもしれない。本来プルリクの目的以外の修正は、別のプルリクで対応するべきではある。
とはいえ、実務でレビューしていて何か問題を見つけた時、その流れで指摘した方が作業効率がいいのは確か。また、プルリクの目的から本当に大きくそれた問題を見つけることはあまりない（そもそもそういう箇所は見ない）。あえて別の修正を依頼するために担当者にプルリク以外の手段で修正を依頼して、そっちで別のプルリクを作ってもらう手間と、その作業の間にうっかり忘れてしまったりするリスクを考えると、見つけたその場で指摘する方が漏れがなくて良いと思っている。

名前については（クラス、メソッド、変数等）、適切でないものは細かくても指摘する

あえて書かなくても当然だが、コードにおいて名前は本当に重要。もちろんここにはコメントの適切さなども含まれる。可読性が低下するとその後のメンテナンス性が低下するのはもちろん、レビューの効率も悪くなる。
あとスペルミスは絶対に修正する。スペルミスがあるとgrepで見逃したりする可能性がある。

mavenで依存ライブラリ一覧をHTML形式で出力する

次のコマンドで、プロジェクトが依存しているライブラリの一覧のHTMLファイルを出力できる。このとき、ライブラリがさらに依存しているライブラリも再帰的に集めてくれる。

$ mvn project-info-reports:dependencies 

IEでの互換表示をX-UA-Compatibleの設定で切り替える

作ったWebサイトをIEで表示確認すると、自動で古いバージョンの互換表示されてしまうことがある。 これは開発者ツールのエミュレーションタブにあるドキュメントモードで確認できる。

たとえばこれを常に最新のモードで表示したい場合、以下のようなmetaタグをheadに追加する。

<meta http-equiv="X-UA-Compatible" content="IE=Edge" />

contentにはこれ以外にも任意のエミュレーションモードを指定するために"IE=7; IE=8"のように設定することもできる。

Spring MVCでThymeleafを使う

Thymeleafについて

ThymeleafはJSPに変わるような、主にHTML向けのテンプレートエンジン。JSPよりも高性能らしい。大きな特徴は、HTMLとしても妥当なマークアップでテンプレートを記述できること。同様のアプローチはSeasarファミリーにMayaaというのがあった。Mayaaはテンプレートのほかに、データとテンプレートのマッピング用のXMLを用意する必要があったのがイマイチだった。Thymeleafは普通にテンプレートファイルは一つで大丈夫。

ここではSTSのSpring MVCのテンプレートで作成したプロジェクトのテンプレートエンジンを、Thymeleafに変更する。

Mavenで依存を解決

pom.xmlのdependencyに以下を追加。

<dependency>
  <groupId>org.thymeleaf</groupId>
  <artifactId>thymeleaf</artifactId>
  <version>2.0.16</version>
</dependency>
<dependency>
  <groupId>org.thymeleaf</groupId>
  <artifactId>thymeleaf-spring3</artifactId>
  <version>2.0.16</version>
</dependency>

で、以下の（あるいは他の）コマンドを実行。

$ mvn install

Springの設定

Springの設定ファイルでテンプレートエンジンをJSPからThymeleafに変更。

<!-- 
<beans:bean class="org.springframework.web.servlet.view.InternalResourceViewResolver">
  <beans:property name="prefix" value="/WEB-INF/views/" />
  <beans:property name="suffix" value=".jsp" />
</beans:bean>
-->

<beans:bean id="templateResolver" class="org.thymeleaf.templateresolver.ServletContextTemplateResolver">
  <beans:property name="prefix" value="/WEB-INF/views/" />
  <beans:property name="suffix" value=".html" />
  <beans:property name="characterEncoding" value="UTF-8" />
  <beans:property name="templateMode" value="HTML5" />
  <beans:property name="cacheable" value="false"/>
</beans:bean>
<beans:bean id="templateEngine" class="org.thymeleaf.spring3.SpringTemplateEngine">
  <beans:property name="templateResolver" ref="templateResolver" />
</beans:bean>
<beans:bean class="org.thymeleaf.spring3.view.ThymeleafViewResolver">
  <beans:property name="characterEncoding" value="UTF-8" />
  <beans:property name="templateEngine" ref="templateEngine" />
</beans:bean>   

注意点としては、開発用にテンプレートのキャッシュを無効にしてある（デフォルトでは有効）。プロダクションにでプロイするときにはこの設定を有効にするべきでしょう。<beans:property name="cacheable" value="false"/>という箇所。

テンプレートを記述

あとはWEB-INF/views以下にテンプレートをThymeleafの書式で書いていくだけ。Spring MVCのコントローラからの呼び出し方法についてはJSPと全く同様。 ただし、テンプレートのHTMLには以下のような宣言が必要。Thymeleaf単体で利用するのとは違い、thymeleafに'-spring3'というサフィックスがつくのに注意。

<!DOCTYPE html SYSTEM "http://www.thymeleaf.org/dtd/xhtml1-strict-thymeleaf-spring3-3.dtd">

<p th:utext="#{home.welcome}">Welcome to our grocery store!</p>

proxyconf 0.1.0 リリース add, useなどのサブコマンドを追加

ターミナルのためのプロキシ切り替えコマンド"proxyconf"の0.1.0をリリースしました。

• Github : proxyconf

このリリースではadd, useなどのサブコマンドが追加され、ターミナルで任意のプロキシに素早く切り替えることができるようになりました。 基本的な使い方はGithubのREADMEと前のブログ投稿をみてください。

アップデート

旧バージョンを使っていた人は以下のコマンドでアップデートできます。

$ gem update proxyconf
$ proxyconf-setup update
$ source "$HOME/.proxyconf/proxyconf"

通常の使い方

システムに設定されているプロキシを使う場合はproxyconfコマンドを実行するだけです。（旧バージョンから変わらず）

$ proxyconf
set proxy: yourproxy.com:8080

これで環境変数のhttp_proxy, https_proxy, ftp_proxyに、今使っているネットワークに対してシステム環境設定で設定しているプロキシが設定されます。

任意のプロキシを設定して使う

addコマンドで任意のプロキシをproxyconfに登録し、useコマンドで設定したプロキシに切り替えることができます。

$ proxyconf add other other.proxy.com:8080
$ proxyconf use other

addで登録したプロキシの一覧はlistコマンドで表示できます。またinfoコマンドで指定したプロキシの設定内容を確認できます。

$ proxyconf list
current
other
$ proxyconf info other
export http_proxy=other.proxy.com:8080
export ftp_proxy=other.proxy.com:8080
export https_proxy=other.proxy.com:8080

登録したプロキシはremoveコマンドで削除できます。

$ proxyconf remove other
$ proxyconf list
current

proxyconf : ターミナルのためのプロキシ切り替えコマンド

proxyconfというターミナルのためのプロキシ切り替えツールをgemでリリースしました。

• Github : proxyconf

いつも仕事でMacを使っているとき、Ethernetケーブルでは会社のネットワークに、WiFiでは別のプロバイダのネットワークに繋がっているんですが、会社のネットワークではプロキシサーバーを通さなければ行けません。Macだとブラウザでネットをみるときにはシステム環境設定で設定してあるプロキシを自動的に使ってくれるので問題ないのですが、ターミナルでcurlのようなネット接続するツールを使うにはいちいち環境変数のhttp_proxyを設定しなければいけないのが面倒でした。

そこでターミナルでも現在のネットワークに対応したプロキシを自動的に設定するツールとしてproxyconfを作りました。

インストール

以下の手順でインストールしてください。

$ gem install proxyconf
$ proxyconf-setup

proxyconf-setupコマンドで、~/.proxyconfディレクトリを作り、~/.bash_profileの最後にproxyconf用の設定を読み込むための一行を追加します。この設定は次にターミナルを開いたときから有効になります。インストールしたターミナルですぐに利用する場合は以下のコマンドをターミナルに入力してください。

$ source "$HOME/.proxyconf/proxyconf"

使い方

単純にproxyconfコマンドを実行するだけです。

$ proxyconf
set proxy: yourproxy.com:8080

これで環境変数のhttp_proxy, https_proxy, ftp_proxyに、今使っているネットワークに対してシステム環境設定で設定しているプロキシが設定されます。

アンインストール

以下のコマンドでファイルを削除します。

$ gem uninstall proxyconf
$ rm -rf ~/.proxyconf

次に、以下の行を~/.bash_profileまたは~/.bashrcから削除してください。

[[ -s "$HOME/.proxyconf/proxyconf" ]] && source "$HOME/.proxyconf/proxyconf"

対応OS

現時点ではMacにしか対応していません。今後、Linuxなら対応可能だと思います。Windowsは多分無理かな。一部bashを使っているので。

bundlerを使ったgemの作り方、公開方法

Rubyのgemパッケージを作成する手順について。bundlerにgem作成を助ける機能がいくつかあるので利用する。

ひな形の作成

bundle gemでひな形を生成する。

$ bundle gem mylib

gemspecは各gemごとに適当に修正する。ひな形を見ればすぐに内容はわかる。

依存ライブラリ

依存ライブラリはGemfileではなくgemspecファイルに記述して、Gemfileからはgemspecを参照するだけにする。

Gemfile

source 'https://rubygems.org'

gemspec

mylib.gemspec

Gem::Specification.new do |spec|
  ...
  spec.add_runtime_dependency "hashie"
  spec.add_runtime_dependency "json"
  spec.add_development_dependency "bundler"
  spec.add_development_dependency "rake"
end

ビルド

gem buildコマンドでビルド。

$ gem build mylib.gemspec

bundleでひな形を作っている場合は、rake buildでpkgフォルダにgemパッケージが作成される。

$ rake build

公開

gem pushコマンドで公開。このとき、rubygems.orgのアカウントが必要。

$ gem push mylib-x.x.x.gem

同様にbundleでひな形を作っている場合は、rake releaseリリースできる

$ rake release