コネヒト開発者ブログ

コネヒト開発者ブログ

コードレビューで心がけている3つのこと【PHPカンファレンス協賛記念ブログ!】

f:id:fortkle:20191129174026p:plain

こんにちは!エンジニアの @fortkle です。

あの伝説のゲーム「メダロット」のスマホゲームのリリース日がついに 2020年1月23日と発表がありました!*1 いまからワクワクしてきましたね!リリースしたらぜひロボトルしましょう!

さて、今回の記事は「コードレビュー」についてです。コネヒトに入社してから早4年、数百のPRをレビューしてきてだんだんと自分の中でコードレビューに対する接し方が定まってきました。今日は私がコードレビューで心がけていることについてご紹介できればと思います。

レビュワーとして

① "既存コード" の 歴史的経緯を素早く紐解く

コードレビューは様々な目的で行われますが、「欠陥・バグを検出すること」「コードの改善」に期待をしていることが多いかと思います。

これらの目的を達成するためには、既存・変更後のコードの実装意図や背景を理解することがとても重要になります。特に長年運用されているプロジェクトでは、前者の「なぜ今の、既存のコードがこのような実装になっているのか」という歴史的経緯を理解することは、不具合を出さずに安全に開発を行っていくためのスキルの1つです。

弊社コネヒトでは、GitHubのPRを使った開発スタイルを採用しているのでコードの意図や背景を理解するには純粋なコードリーディングと同じぐらい、過去のPRを見るのが効果的なケースがあります。

ここでは、過去のPRを素早く簡単に探し出すために、実際に私が使っているtipsを2つご紹介します。

方法1. PhpStormのAnnotate + Find Pull Request Plugins

  1. PhpStormでAnnnotateを表示
  2. 確認したい行を右クリックし、 Annotate Previous Revision で追加・編集されたコミットまで歴史を遡る
  3. Find Pull Request Plugin でそのコミットが追加されたPRを開く

という手順で簡単にPRを探し出すことができます。

PhpStormやGoLandなどJetBrains製品のエディタにはAnnotateというgit blameと同等の機能があります。行番号を右クリックすることで表示されるようになります。

f:id:fortkle:20191129123840p:plain
Annotate機能

また、Annotate部分を右クリックすると、さらに選択行のコミットの操作ができるようになります。 Annotate RevisionAnnotate Previous Revision を駆使すると、コードスタイルの変更などで追いづらくなってしまったコードも簡単に歴史を遡って探すことができます。

f:id:fortkle:20191129124314p:plain
選択された行をコミット単位で遡れる

最後に、この Find Pull Request というプラグインを利用すると、右クリックしてメニューを選ぶだけで、選択行が追加されたPRを開くことができるようになります。

plugins.jetbrains.com

https://raw.githubusercontent.com/shiraji/find-pull-request/master/website/images/screenshot.gif
https://plugins.jetbrains.com/plugin/8262-find-pull-request から引用

この方法はこちらのブログで紹介されており、大変参考になりました! tenkoma.hatenablog.com

方法2. tig blame + open-pull-request

次に tig と open-pull-request を使う方法です。

  1. tig blame {filename} で確認したい行があるファイルを開く
  2. , < と打つと選択行の履歴を遡れるので追加・編集されたコミットを探す
  3. P と打って open-pull-requestをtig上から実行し、PRを開く

という手順で簡単にPRを探し出すことができます。

方法1のやり方を知る前はこの方法でPRを探していました。 特別なエディタが不要でどんなプロジェクトにも対応できるので今でもたまに利用しています。

f:id:fortkle:20191129143258g:plain
tig blame + open-pull-request でPRを探すイメージ

この方法も設定方法などがこちらのブログで紹介されており、大変参考になりました! blog.mknkisk.com

② PR上でのコメントで工夫する

PRのコメントでもいくつか工夫することを心がけています。

ラベルを使う

PRにコメントを残すとき、私は以下のようなラベルを使うことが多いです。

ラベル 意味
[MUST] 必ず直してほしいとき
[IMO] 自分ならこう書くかな?どう?という緩やかな指摘のとき(In My Opnion)
[nits] 再レビュー不要な細かな指摘(typoやスタイル等)のとき
[質問] 分からないとき、確認したいとき

f:id:fortkle:20191129145823p:plain
ラベル付きのコメント

必要に応じてコメントの先頭にこれらのラベルをつけることで、例えば [MUST] は必ず修正してほしい、 [IMO] で指摘された点は一度考えてもらって採用するかしないか決めてほしい、などレビュイー自身に最終的な変更の意思決定をしてもらえるように考えて使っています。

良い点もコメントする

以前は問題がある点や確認したい点のコメントしか残していなかったのですが、最近は勉強になったときや感謝の気持ちを込めて良いと思った箇所にノイズにならない程度にコメントを残すようにしています。

コードレビューという場を、リスクや課題を見つけ出すことだけでなく、自身の学びや自信につながるポジティブな場として活用するために意味はあるかなと思っています。

f:id:fortkle:20191129144805p:plain
感謝のコメント!

③ とにかく動かしてみる

基本的にPRをレビューするときはどんなに些細な変更でもアプリケーションコードにdiffが入ったら実際にローカル環境で動かすようにしています。

変更内容にロジックが含まれるのであれば、レビュイーが様々なパターンを考慮に入れて実装できているか、抜け漏れがないか、を境界値やEmptyなstateを使って動かして確認します。 このとき単体テストがあれば簡単にチェックができるので、テストの有り難みを感じられますね!

レビュイーとして

続いてレビュイーとして心がけていることを紹介します。

... といいましたがです!

レビュイーとして心がけていることは、今週末の2019年12月1日(日)に開催されるPHPカンファレンス東京 2019でLTとして発表します!

高野福晃(@fortkle) 17:20〜 Track 1 (1F 大展示ホール) Lightning Talks
登壇タイトル:「余裕を生み出すコードレビュー」

当日ご参加される方はぜひ発表を聞いていただければ幸いです!

また、今年もシルバースポンサーとして協賛いたします。 ブースも出しますのでそちらにもぜひお立ち寄りください!

phpcon.php.gr.jp

”Twemoji" by Twitter, Inc and other contributors is licensed under CC-BY 4.0

PR

こんなハートフルなレビューのコネヒトでは一緒に働いてくれるエンジニア募集中です!興味があればこちらもみてください!

www.wantedly.com