xkcdより

概要

コードレビューもスキルなので向上させばさせるほど結果が出ます。
レビューのスキルをあげるために以下を意識してレビューを行います。

参考

以下の記事から一部を引用しています。
https://css-tricks.com/code-review-etiquette/
https://qiita.com/necojackarc/items/0efa125ce1ea90e739f0

コードレビューの目的

「テストとコードレビューの目的」から引用（一部省略）

• ソフトウェア品質の向上および属人性の排除
• スキルの向上及びナレッジの共有
• 担当者の実装レベルの把握と向上（教育的観点）
• 他人のコードを読むことによって学びや発見を得ること
• 開発者同士のコミュニケーションの活発化

意識すること

レビューするのは人じゃなくてソース

NG

ここでリターンしてください。
教科書が会社においてあるのでもう少し勉強しといてください。

• 人をレビューしている
• 勉強してって言われても具体的に何を勉強すればいいか伝えられていない。次のアクションがわからない。
• そこでリターンする意味が伝えられていない。

GOOD

こういう書き方だとstuffがすでに処理済みの時はcalculateStuff()まで行かないで
リターンするので不必要な処理が省かれます

  if (stuffAlreadyProcessed) return stuff
  // process stuff
  return processedStuff

• ソースをレビューしてる
• どうすればいいか伝わる
• なんでそれをやるか伝わる
• サンプルもある

レビューをもらっている人が【何をやるか】【なんでそれをやるか】【次のアクション】がわかるように

NG

ここでapi使えばどう？

• どのapi？
• なんで？

GOOD

Apache CommonsのListUtilsに
public static <T> List<List<T>> partition(List<T> list, int size)
というメソッドがあります！

[ドキュメントのリンク]

APIを使ったら「こっちでソース・ドキュメントをメンテしなくていい」というメリットがありますが、
導入してみたらどうですか？APIドキュメントを読んでも使い方がわからなかったら私に聞いてくださいー

• どのapiのことか・どの関数のことかわかる
• ドキュメントのリンクもある
• apiを使うメリットが伝わる
• 次のアクションがわかる
  • ドキュメント読んでライブラリーを追加してソースを修正する。
  • 一人でわからない場合はレビューを行なった人に聞く

自分が勉強になったこと・面白かったことも

コードレビューはQAでありながら開発者同士のコミュニケーションでもありますので指摘だけじゃなくて

僕はいつもこういうの自分で一から実装しますがapiありましたね！
(apiのリンク)
勉強になりました！

最初ソースわからなかったから調べてみたんですけどscalaのここがめっちゃ便利ですね。
(ドキュメントのリンク)
ありがとうございます！

僕もちょうどこういうの作ろうとしてるんですが中々実装ができなくて。。
(コミット・diffなどのリンク)
あとで相談させてください！

も書きます。

いろんなやり方があるって意識する

NG

Library Aを使ったほうがいいです。

• 事実と意見を区別できてない

GOOD

処理がはやくてapiドキュメントも充実してるので僕はいつもLibrary Aを使うんだけど
Library Bはどんな感じですか？

• Aのメリットを伝える
• Bのメリットを聞く
• Aのほうが本当にBよりいいのか判断しやすくなる

2018-10-24 追加

レビューの指摘コメントにタグをつける

コードレビューを効率化するちょっとした工夫を読んでタグの使い方とメリットを覚えました。詳細はリンク先の記事を読んでいただきたいですが、ざっくりいうと

• レビューの指摘コメントにラベルをつけることでコメント対応の優先度をつけたりできます。
  • [MUST] 必ず直すべきです。
  • [IMO] 私はこう思います。こうした方が良いのでは？ 意見や緩やかな指摘。
  • [nits]ほんの小さな指摘。インデントミスなど細かいところに。
• 具体的な使い方と他のメリットは コードレビューを効率化するちょっとした工夫をご参照ください。

自動コードフォーマットツールを導入

• まわりにいるエンジニアに「コードレビューどうしてるの？」と聞いてみたら「自動コードフォーマットツールも入れたほうがいいよー」と教えていただきました。
  • たとえばScalaだったらScalafmt
  • コードのインデントなど自動的に直してくれるのでコードレビューのノイズになりがちな[nits]が減ります
  • ツールができる仕事をツールに任せることで人間の仕事に集中できる時間が増えます