見出し画像

homieのコードレビューを覗き見してみませんか?

はじめに

はじめまして。
homie株式会社でエンジニアとして働いている片山と申します。

今回はチーム開発においては欠かせないPRレビューについて、個人的な解釈も含めてhomieのレビュー文化を紹介いたします!

これを一つのサンプルとして、PRレビューに携わるみなさんにとっての参考になれば嬉しいです。

P.S. 最近グリーンカレーにハマっているのですが、2週くらい回っていなばのグリーンカレーはクオリティが高いことに気づきました笑

レベルの高いグリーンカレーをオールウェイズ出してくれるいなばさん

コードレビューの必要性

そもそも、なぜコードレビューは必要なのでしょうか?
私はhomieでのレビューを経験して、現在は以下のような考えになっています。

  • メンバー間でコーディングスタイルやドメイン知識を共有することで、チーム全体で一貫性のあるコードを実現する

  • コードの品質を担保し、安全性やメンテナンス性を向上させる

  • 技術の新しい解釈やベストプラクティスに関する知識を共有し、チーム全体の技術力を向上させる

  • レビュー時に必要に駆られて仕様を追うことで、仕様の理解を深める

次に、homieのコードレビューの特徴を紹介します。少しでも雰囲気が伝われば幸いです。

homieのコードレビューの紹介

PRの粒度は小さく

タスクを小さなPRに分割し、レビュー依頼をすることを推奨しています。

小さく分割することで、他機能から独立しているPRは早めにマージできたり、レビュワーが確認するべき差分を少なくしてレビュー時の負担を軽減できるといったメリットがあります。

PRコメントはテンプレート化

新規参画したメンバーがレビュー依頼時にどういう情報が必要なのか理解できるように、PRコメントにはテンプレートを使用しています。

また、要件や設計ドキュメントへのリンクを共有することで、レビュワーが仕様を把握しやすい状態を維持しています。

PRコメントのサンプル

レビュー依頼のリマインド

homieではレビュー忘れが発生しないように、以下のようなGitのリマインド設定を推奨しています。
スケジュールされたリマインダーの管理 - GitHub Docs

この設定を行うことで、自分がレビュワーにアサインされているPRを毎日定時に通知してくれます。

レビュー忘れが頻発し、PRがマージされないまま滞留している状態だと開発スピードが落ちるうえに、時間が空いてしまってPRの内容を再度キャッチアップするコストが発生するため、なるべくfast-mergeを推奨しています。

ちなみに、上記の理由から自分のチームではなるべく2営業日以内でのレビューを行うルールがあったりします。

構文エラーやフォーマット違反はレビュー前に

フォーマット忘れや静的解析等はCI上で実行して、レビュー依頼前に検知できるような取り組みをしています。

この取り組みの理由としては、正解がある程度決まっているものに対しては時間を使わずより本質的な議論に時間を使うためです。

加えて、軽微な指摘が増えるとPR上の見通しが悪くなり、重要な指摘が埋もれてしまうリスクもあります。

軽微な修正漏れはレビュー前に気づけるように

READMEやレビュールールの充実

新規参画したメンバーがレビュー時に困らないように、汎用的や仕様はREADMEに、過去のレビュー事例はレビュールールにまとめて適宜更新をしています。

レビュールールでは技術的な事項だけでなく、レビューするときにより円滑なコミュニケーションをとるためのアドバイスなども含まれています。

メンバーにより順次更新中です

社内勉強会

レビューに直接関わるわけではないのですが、メンバーが有志で企画した社内勉強会で、レビュー関連の発表が行われることもあります。

以下は10月の勉強会資料の一部です!
実務のなかでレビューする時間は少なくないはずなので、メンバーの温度感も高そうですね!

毎月有志の勉強会が開催されています

勉強目的のレビュー・質問は大歓迎

自チーム以外のPRであっても勉強目的でレビューしたりといった取り組みは大歓迎で、homieにおいてもメインのレビュワーと別で、有志で積極的にサブレビュワーを立候補してくれるメンバーもいます。

個人的には他チームからのレビューは大きな意味があると考えていて、
立場の違う目線からコードを見ることで、潜在的な思い込みがない状態でレビューすることができるため、実装者が気づけなかった不具合を事前に検知できたり、より事前知識がない状態でも読みやすいコードを実現できたりといった意味があるのではないかと実感しています。

レビュー内容

ここまではレビューの全体像の話が中心だったと思うのですが、次はhomieで行われているレビューの観点について紹介します!

リファクタリングには感謝を!

弊社ではボーイスカウトルールを採用していて、自分の担当外であっても有志で軽微なリファクタリングをすることを推奨しています。(影響範囲の特定や、テストの追加などは前提として)

ボーイスカウトは「来たときよりも、美しく」の理念に基づいています

個人的には、コードはチームの共有財産なので、メンバー同士が少しずつコードをきれいにしていくことで積もり積もって全体として一貫性のあるシステムになるのではないかと考えています。

命名について

関数や変数の命名についてのレビューコメントは多いです。

UnnecessaryNotificationという命名についての指摘

個人的にも関数や変数の命名には、大きな意味があると認識しています。

コードリーディングにおいて、限られた時間の中で知りたい情報を得るためには、どの処理を読むべきかの優先順位付けが重要だと考えています。

その観点で考えると、関数名で過不足なくふるまいを説明できていることは読まなくてよい処理を判別するために大きなメリットを持っています。

1つの関数にいろんなふるまいを詰め込まない、副作用があるかどうか変数名や引数でわかるように、チームで命名の規約があるのであれば参照するなど、基本的なことではありますが年々大切さを身をもって実感しています笑

HOTLEADに関する質問

レビューは指摘だけでなく、疑問を解消していろんな知識や考え方をインプットする場でもあると考えています。

homieにおいても、HOTLEADのアーキテクチャや使用されている技術に関する質問や、ドメイン部分の理解を深める質問など、多種多様な質問がPR内で行われています。

DDDに関する質問

その他

上述した内容以外にも、パフォーマンス・セキュリティの観点や仕様の理解を深める議論など、バラエティに富んだレビューが行われています。

おわりに

今回はPRレビューについて、個人的な解釈も含めてhomieのレビュー文化を紹介させていただきました。雰囲気がなんとなくでも伝われば嬉しいです!

あくまで一つの事例でしかないのですが、少しでもここまで見ていただいたみなさんの参考になれば幸いです。

ここまで見てくださったあなたにLGTM

homieってどんな会社なの?と思ったそこのあなた・・・

homieは個人の成長が事業成長に繋がると考えています。
そのためhomieの福利厚生では、技術書の購入や社外セミナーの受講に加え、SaaS(Miro, AWS, GCP, ChatGPT等)の利用も可能です✨

成長の機会に溢れているので、ギークなメンバーとギークなお話をしたい方、事業内容や働き方などにご興味がある方はお気軽にお話ししましょう♪

この記事が参加している募集

オープン社内報

みんなにも読んでほしいですか?

オススメした記事はフォロワーのタイムラインに表示されます!