21
8

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

Ninja17Advent Calendar 2017

Day 1

僕がコードレビューをもらうときに意識していること

Last updated at Posted at 2017-11-30

はじめに

Progate Advent Calendar 2017 初日の記事です:point_up:
会社もサービスも少しずつ成長してきて、やっとAdvent Calendarがやれるくらいの人数にはなってきました(まだ25人もいないけど)。

この記事では、僕がコードレビューをもらう時に意識してる(割と当たり前な)ことをゆるく書きました。
「レビューするコツ」みたいな記事はよく見かけますが、「レビューもらうコツ」はたまにしか見ないので。

ー ー ー ー ー ー ー ー ー ー ー

コードレビューをする意義として、

  • コードの品質の維持(向上)
  • 個人の技術力の向上

があるかな、と思います。

1つ目の方はプロジェクトや組織全体に関わる話なので、忘れられず大事にされ続けるケースが多いと思います。

2つ目の方は組織によっては粗末にされがちのようです。
でも個々の成長こそがプロジェクト全体の成長に直結するので、これもまた大事なことです。

僕はコードレビューをしてもらえるときがとても幸せです。
先輩だけでなく、同期や後輩からも様々なレビューをもらえて、自分の技術力の幅を広げることに役立っています。
(以前はエンジニア1人で働いてたこともあったので、尚更ありがたみを感じます。)

今回は自分がコードレビューをもらう時に意識していることが4つくら思いついたので順に書いていきます。

1. 自分で自分のコードをレビューする

まずはコードレビューを依頼する前に、自分で一通りちゃんとチェックするようにしています。
その際に特に意識しているのは、

  • タイポしてないか
  • 明らかに不要なコードがないか

などです。
僕の場合はデバッグ用のブレークポイントや出力をそのまま放置してることが多々あるので。

レビュアーからこれらのようなケアレスミスへの指摘を受けるのは、非常にもったいないと個人的には思ってます。
せっかくのコードレビューなので、もっと重要なところに思う存分時間を使ってもらいたい、というところです。
なので、事前に自分で防ぐ努力はしているつもりです。

あとは似たような話で、Lintやテストは事前に実行して確認するのは忘れずにやりたいですね。

2. 開発の背景を明記する

新機能の作成・既存システムの改善・バグの修正、など、開発の内容によって様々ではありますが、いずれにしてもその開発を行った背景は存在すると思います。
当たり前なことなのかもしれませんが、その背景をちゃんと書くことを大切にしています。

スクリーンショット 2017-11-30 21.30.07.png

↑ これはかなりシンプルな例ですが、少しでもあるだけで効果は大きいです。

僕が思う、あんまり良くないコードレビューの1つに「コードだけをレビューする」というものがあります。
コードの差分だけをみて、コードの見た目から得られる情報だけをレビューする。
これだけではやっぱり不十分で、どういう経緯でそのコードを書いたのか、みたいな部分までレビューできるのが理想だと思ってます。

これは当然レビュアー側の意識の問題でもあるんですけど、もらう側にもできることはたくさんあると感じています。
深いところまでレビューをもらえるような工夫は積極的にしていくべきだと思います。

3. 見てほしい箇所を明記する

例えば50行ある変更の中でも、キモになってる機能や実装に迷った箇所など、特に見てほしい部分はあったりすると思います。

  • この関数内のロジックがめっちゃ大事
  • テーブル構成がやや不安
  • 地味にここの変数名に迷った

プログラミングの力は難題を乗り越えたり、あやふやな点を解決していくことで伸びていくと思ってます。
レビュアーと一緒に今直面している「大事なこと」に集中して向かっていければ成長も加速するはずです。

4. 見る必要のない箇所を明記する

先ほどの逆で、見る必要のない箇所があればちゃんと書くようにしてます。

うちの開発体制の場合、ほぼ全ての変更はレビューを受ける必要があることになってます。
ですが、当然あまりレビューが必要ない変更もあります。

  • タイポ直しただけ
  • 他の場所から移動させただけ
  • 誰になんと言われようと明らかに間違ってない(?)

などなど、です。

先日、メンバーのプルリクを眺めていたらこんなコメントがありました。
スクリーンショット 2017-11-28 10.51.08.png
レビューが不要な箇所を明示することで、レビュアーが楽になるだけでなく、本当に大事な箇所だけに集中してレビューすることができて幸せですね。

最後に

まぁ割と当たり前だな、と思った方も多いような内容かな、と思います。
でも現場では〆切とかプレッシャーとか深刻なバグによる精神崩壊とかで当たり前のことが当たり前にできなくなるものだな(仕方ないんだけどね)と思ってます。

あと、コードレビューって、どうしてもシビアな感じになりがちです。
でも、複数人で1つのものを作っていくのはとても楽しいことで、コードレビューも楽しむべきだ、と常々思ってます。
殺伐とした感じではなく、みんなが楽しく会話してる感じのプルリクエストが大好きです。

今度は(もうちょっとしっかり)、レビューする時に意識してることも書きたいと思ってます:thinking:

ー ー ー ー ー ー ー ー ー ー ー

ちなみに Progate Advent Calendar 2017 の明日の担当は社長さんです!

21
8
1

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
21
8

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?