LoginSignup
27
5

More than 1 year has passed since last update.

内定者インターン生が贈る!初心者向けコードレビューの手引き

Last updated at Posted at 2022-12-05

はじめに

LITALICO Advent Calendar 2022 カレンダー3の6日目の記事です

はじめまして! 今年の6月から内定者インターンをしている@marie-yです。この記事では、業務未経験だったインターン生がコードレビューする中でやっていたこと、意識していたことについて書きます。初記事なので、至らない点はあるかと思いますが、ぜひ読んでいただけると嬉しいです!

今回、コードレビューについて触れていきますが、チームごとに方針等も変わると思うので、全てのコードレビューする方に当てはまるわけではないことをご了承ください。

この記事の対象読者と目的

この記事の対象読者は、コードレビューに不慣れな人です。私は、コードレビューをしたことがありませんでしたので、「コードレビューって何? どうすればいいの?」という目線から、私なりの考えをお伝えしていきます。「絶対にこうすべき!」という訳ではないので、参考にしていただけたらと思います。

コードレビューの目的

私が内定者インターン生としてチームに配属され、初めてのコードレビューでまず最初に思ったことは、

:thinking: (コードレビューって、先輩のコードが間違っていないか私が見るの? そんなのわからないし、先輩が書いたコードだから合ってるんじゃ...?)

です笑 私が、コードレビューに悩んだ大きな要因の1つは、このコードレビューの目的の捉え方を間違えていたからだと思います。

コードレビューを始めた当初は、コードレビューの目的を他の人が書いたコードに間違いがないかや、より良い書き方がないかを確認することだけだと思っていました。もちろんそれも大切なのですが、それだと初心者からすると少しハードルが高いです :confounded:

私が現在考えているコードレビューの目的は、他の人が書いたコードに対して、チームで共通認識を持つことです。もう少し具体的に言うと、アプリケーションとコードにどんな変更があったのかを理解することだと思っています。

ここで私が言っている、アプリケーションの変更とはユーザーから見た変更を意味します。例えば、画面が変わったとか、新しい機能が追加されたなどです。
コードの変更とはエンジニアから見た変更を意味します。例えば、新しいモジュールが作られたとか、新しいビューが作成されたなどです。この2つの観点の変更を理解することがコードレビューの目的だと考えています。

「どんな変更があったのかを理解するだけでいいの?」と思うかもしれませんが、どんな変更があったかを理解するのだけでも結構苦労します笑 そして、どんな変更があったのかを理解する過程で質問することによって、バグ発見などにつながっていきます。

初心者向けコードレビュー手順

アプリケーションとコードにどんな変更があったのかを理解するための、私のコードレビュー手順はシンプルです。

  1. PRの説明を読んで、アプリケーションの変更を確認する
  2. ブランチをローカルに落としてくる
  3. 動作確認を行って、アプリケーションの変更を理解する
  4. 調べたり質問したりしながら、コードの変更を理解する

初心者向けコードレビューのポイントは、コードを読むよりも先に動作確認を行うことです。初心者にとっては、アプリケーションの変更の方がコードの変更よりも理解しやすいです。そのため、この順序で行うことで、ある程度コードが理解しやすくなります。

それでは、それぞれの項目について詳しく説明していきます。

1. PRの説明を読んで、アプリケーションの変更を確認する

最初に、PRの説明を読んで、アプリケーションの変更を確認します。アプリケーションの変更というのを具体的に挙げると以下のようなことです。

  • 変更するページはどこか? (ex)トップページ、一覧ページ、詳細ページなど
  • 変更する内容は何か? (ex)ボタンの位置や色を修正する、機能を修正する、バグを修正するなど

2. ブランチをローカルに落としてくる

PRの説明を読んで、アプリケーションの変更を確認したら、次にブランチをローカルに落とします。ブランチをローカルに落とす方法は、例えば、レビューしたいPRのブランチ名がhogeだったとき、以下の手順で取り込むのがおそらく一番早いです。

// リモートリポジトリの全てのブランチの変更をリモート追跡ブランチに取り込む
git fetch 

// PRのブランチに切り替える
git checkout hoge

リモート追跡ブランチとは、「ローカルリポジトリにある、リモートに前回接続した時のコミットを参照しているブランチ」です。私もうまく説明できないので、詳しくは、こちら等を読んでみるのがいいと思います :point_down:
(丸投げごめんなさい :bow:
3.5 Git のブランチ機能 - リモートブランチ

3. 動作確認を行って、アプリケーションの変更を理解する

ブランチをローカルに落としてきたら、動作確認を行って、アプリケーションの変更を体験しながら理解します。PR作成者が動作確認項目を書いてくれている場合は、その項目をチェックします。書いてない場合や、足りないと思う部分がある場合は、自分でチェックリストを作成します。

自分で作成したチェックリストが不安な場合は、GitHubのコメントでPR作成者に聞きます。そのチェックリストをPR内にコメントで残しておくと、様々な点でいい(以下参照)ので、必ず残しておきます。

  • 自分が何を確認したのか忘れない
  • PR作成者が見てもらったポイントを確認できる
  • 誰かが動作確認項目を残してくれていると、他のレビュワーも助かる(他の人がどんなポイントで見ているのかが知れると勉強になります :pencil:
  • そのPRによってバグが発生した時に、なぜ動作確認項目で気づけなかったのかを考える根拠になる

4. 調べたり質問したりしながら、コードの変更を理解する

初心者がコードレビューを始めて、最も時間がかかるのはここだと思います。私も、これに関しては自分なりの方法を確立できているわけではないので、やっていることと、質問する時に気を付けていることをお伝えします。

コードはGitHubではなくエディタで見る

コードレビューを始める時は、GitHubを見ているのでコードの変更もGitHub上で見てしまいがちです。しかし、ここに落とし穴があります。GitHub上で見るよりも、普段コードを読んでいる自分のエディタで見る方が断然読みやすいです。エディタ上で見た方が、コードジャンプ機能が使えたり、検索がしやすかったり、(おそらくエディタでカスタマイズした)自分の見やすい文字の大きさ、フォントで見れたり等、メリットがたくさんあります。

 「エディタで見てたら変更差分がわからないじゃないか!」と思うかもしれませんが、VSCodeでは便利な拡張機能があるので、そちらを使うことをおすすめします。

上記を使えば、GitHub上でコードを見るのと同じように、変更差分を見ることができます。私も最近、先輩エンジニアから教えていただいたので、まだまだ使いこなせていないです。改めて説明動画を見てみたら、checkout機能もここに搭載されているので、わざわざターミナルに行かなくてもcheckoutできるみたいですね :eyes:

わからない箇所は、変数の中身を見る

コードを読んでいると、ここ何やっているんだろう?と思うところがありますよね。そういう時は、変数の中身を見ると、何をやっているのかがわかりやすくなります。そのため、それぞれの言語のデバッグツールを使って、変数の中身を見てみることが大事です。私が主に使っているのは、RailsとReactなので、binding.prywindow.console.logを使っています。また、Reactのコンポーネントを確認するときには、React Developer Toolsを使っています。他にも良いツール募集中です :pray:

自分の使っている言語に合わせて調べてみたり、先輩に聞いてみたりしてください。

質問する時は、できるだけ具体的に質問する

質問する時に気を付けていることは、自分が聞きたい範囲が伝わるように、自分の考えを具体的に伝えることです。初心者だと、右も左もわからないということが多々あるため、自分なりの考えを伝えるのも難しいと思います。ですが、先輩方には色々なものが見えているため、抽象的な質問だと、様々な答えが考えられ、困ってしまったり、どこまで聞きたいのかが分からなかったりします。

悪い例だとこんなのがあります。以下のようなRubyで書かれたモデル(架空)があった時に、

class Post
    has_one :special_post
    # Postsの定義内容
end

class SpecialPost
    belongs_to :post
    # SpecialPostsの定義内容
end

質問者 :person_frowning:SpecialPostって何でしょうか?」
回答者 :person_frowning_tone1: 「(SpecialPostのどこがわからないのかな。。。)」

SpecialPostって何だろう?って思うので、確かにそう聞きたい気持ちもわかりますが、これだと回答者は、SpecialPostの何が分からないのか分からないので、一からSpecialPostについて伝えなくてはいけません。これは、質問者も求めていないはずです。

そのため、以下のように具体的に質問する方が好ましいです。

質問者 :person_frowning:PostSpecialPostの違いって何でしょうか? クラスの継承はしていなくて、1対1の関係であることは理解できました。この2つにはどのようなデータの違いがあるのでしょうか?
回答者 :person_frowning_tone1: 「(なるほど、PostSpecialPostのデータの違いがわからないのか!)PostSpecialPostのデータの違いは、、、」

というように、回答者も質問者の意図を捉えやすくなります。そのため、質問する際は、自分の考えていることを具体的に伝えるようにすると良いと思います。

自分の考えていることを具体的に伝えるのも最初は中々難しいですが、言語化している中で自己解決することもあるので、訓練することがおすすめです。私もまだまだなので、訓練していきます :muscle:

最後に

初心者向けコードレビューの手引き、いかがでしたでしょうか? 私は自分で言語化していく中で、今まで何となく行っていたレビューというものを、振り返るきっかけになりました。最後まで読んでくださりありがとうございます :cat:

27
5
0

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
27
5