Help us understand the problem. What is going on with this article?

ソースコードレビュー戦略

Fringe81のAdvent Calendar最終日でございます。
本稿では僕が「ソースコードレビュアー」という役割となったときにどんなことを考えているかを、だいぶ前提のことを中心にツラツラと書いてみます。

開発者として生活していると、いろいろな局面で『レビューいっちょうオネシャス的』な感じでプルリクを受け取ったりすることもあるんですが、
何をどうレビューするのかということがふんわりとしたまま、膨大なソースコードを渡されたときなど、軽く途方にくれることなどがあります(笑)。

これは僕がフリーランスで様々なプロジェクトに携わっていたときや、社外のエンジニアさんとなにか共同作業するときなどにずっと思っていたことなんですが、当たり前のレビュー前処理(どのようにレビューするのか)が、意外となされていないことが多いような印象がありました。
じゃあ何をどうレビューできれば良いのか?ということに対して自覚的になり、そのための行動を採ることができれば、少し幸せになれるんではないかなどと思った次第です。

1.レビューの役割

まずは、レビューするという過程においてレビュアーたる自分は何をなすのか?どんな事を意識しているのか?という超前提っぽいところから整理してみます。

1-1.何をなすのか:ポジティブな変更のきっかけを作る

僕はレビューの捉え方として「ポジティブな変化を作るもの」という感覚を持っています。ではレビューの前後で変化するものとは何でしょうか?

  • レビュイーの解釈:レビュー時の指摘によってレビュイーが新たな観点を持つようになる
  • ソースコードそのもの:レビュイーがソースコードを書き換える
  • チームのポリシー:議論がきっかけでチームの指針がアップデート、周知される

などが考えられるかなと思います。
ではそのコミュニケーションの中身はどのようなものであるべきなんでしょうか?

名称未設定のノート (2)-2.jpg

1-2.何を意識するのか:時間対効果

レビューの中身がどんなものであると良いのか?
レビューを行える時間は有限です。
仮に時間が無限であるならば、人間コンパイラーとしてエラーになるコードを検出できるかもしれません。
あるいは、レビュイーにガツッとインタビューして、レビュイーの体験をトレースすることで細かい改善策を提示することもできるかもしれません。

ここではHOWの話はいったん置いておいて、僕が意識していることは単純なことですが、「かけられる時間に対して、得られる利得をなるべく大きくすること」です。
同じ時間を使うなら、なるべくプロダクト/チーム/レビュイーにとって、大きな利得が生まれることにフォーカスする、という考え方です。利得を考える際の頭の中の優先順位づけは状況によってまちまちではあります。

名称未設定のノート (2)-3.jpg

2.レビュイーと戦略を共有する

ではどんなレビューができると時間対効果が大きいのでしょうか。僕がまず間違い無いと思っているのは目的がフワッとしたままに行うレビューは成果が少なくなりがちだと思います。

そもそも開発者たる我々は、どのような目的を持ってソースコードレビューを行うんでしょうか?
実はそこは複雑な世界で、様々な思惑が入りやすく、クリアなレビュー戦略を立てないと、レビューの場はわりとボンヤリとした場になりやすいのではないかと思います。

2-1.論点を明確にすること

時に初対面に近いレビュイーとの関係で「コードみてください」と言われる状況もあるのですが、レビュアーからすると何を論点として注目すべきか、その可能性はたくさんあります。

  • 要件に関する話
  • ドメインに関する話
  • 設計に関する話
  • 可読性に関する話
  • パフォーマンスに関する話
  • 実装ポリシーに関する話
  • コードフリーズまでの時間的猶予を鑑みての現実的な落とし込み
  • 育成の観点(アドバイスや指摘)

ここで「合点承知!」となんとなくコードをみて、自分目線で気づいたことをコメントすることもできるのですが、ここで一つ踏みとどまることでチャンスが得られるかもしれません。
時間対効果の高い議論は何なのか?ということをレビュイーと握ること。
議論すべき効果の大きい議題が何なのか?を探ることは大事だと思います。ここでやっぱり大事なのは対話だと思います。質問しながら要点はどこにあるのかを探り当てていくイメージです。
レビュイーの方で要点を整理してくれている場合もありますが、影響範囲の大きそうな変更時には隠れた課題はないか?という事も注意してみたり。

ここで論点を明確にして効果の高い議論を中心に行うとよいのではないでしょうか。

名称未設定のノート (2)-4.jpg

2-2.レビュイーに論点を提示してもらう

何度かレビュイーの仲間に論点を明確化することをつたえつづけていたところ、レビュイー本人が相談事を予めプルリク内にコメントづけしてくれたことがありました。
そうしてもらうことでレビュアーとして考えるべきことの濃淡をつけやすくなった経験があります。
レビュイーとしてもちょっとした一手間をかけることで、見てほしい問題をスルーされることなく議論ができるので、このようなやり方もいいんじゃないかと思います。

名称未設定のノート (2)-5.jpg

2-3.文脈の構築

論点を明確にする上で、助けになってくれるのがアーキテクチャ、コード規約、チームルールなどではないでしょうか。
レビュイーや開発チームと前提となる文脈を構築しておくと、共通理解が生まれ、そもそも的な話を省略でき、より本質的な問題にフォーカスすることができるようになります。
僕が直近いたプロジェクトでは下記のようなトライをしていました。

  • プロジェクト全体でDDDを実践することとし、ドメインに関する知識は全員が持っていることを前提とした。
  • アーキテクチャの思想をドキュメント化し、マークダウンで保存。作者が自分であることを明記して、相談事があれば話をしに来てもらう。変更がある場合はレビューを経て公式にアップデート。
  • 上記のアーキテクチャを加味して、詳細設計+実装における推奨手順をドキュメント化。レビューを行うタイミングも何箇所か提示。
  • レビュールールの共有。レビュー時には整えておいてほしいコードの期待品質(単体テストは実施済み等)。レビュアーが見ること/見ないことの明記。オフラインレビューを実施したいケースの明記。

名称未設定のノート (2)-6.jpg

2-4.論点の因数分解

論点は一つなのかと思って、よくよく話を聞いていくと、複数の話が混在していたりすることもよくあります。
因数分解が可能な問題は、分解しないと問題や解の評価が複雑になってしまうし、取り組まなくていい問題にまで時間を割くはめになってしまったりします。
「この問題は、このにこのバージョンで対応しなくても、次回のバージョンで吸収できるね」なんてことも割とありました。
因数分解できる論点は?という事をレビューの前にレビュイーが考えるような習慣、文化づくりも一つの文脈構築の仕方ですね。

名称未設定のノート (2)-7.jpg

3.コミュニケーション設計

ここから先はオマケ的な感じなのですが、論点を明確にしたり、ソースコード改良のための議論を行っていく上でどのようなコミュニケーションを設計するのが良いのでしょうか?
自分なりに考えていることなどを書いてみたいと思います。

3-1.オフライン/オンラインの使い分け

レビュイーの気持ちを考えてみると、オンラインでメッセージやプルリクを出して終わり!(あとは察して!)という流れでうまくいくなら楽だし良いですよね。レビュイーの立場からするとレビュアーの時間を奪ってはいけない、レビュアーの都合のいい時に見てほしいという心理も働くのかもしれません。
もちろん、それでレビュアーがうまく低コストで察することができるならば全く問題ないと思いますが、やっぱり論点が曖昧になりがちです。忙しくてオンラインレビューを後回しにした場合にレビューを忘れてしまったりといった失敗も😓
オフラインで直接対話をするか否かを判断するとき、僕が考えている判断材料はキャッチボールの回数、時間的猶予の有無です。
ポンポンと意見交換をしながら、自分の振舞いを分岐させて対話をすすめるような場合や、時間がないのでとにかく要点を共有しつつ質の高い状態に持っていきたいような場合はオフライン対話を行うようにしています。
なんだかんだでオフラインで直接対話できる状況は強いと思います。お絵かきで思考を伝えたり、第三者を巻き込んだりすることもしやすかったり、選択肢の幅が大きく広がるのではないでしょうか。

名称未設定のノート (2)-8.jpg

3-2.便利な言葉「迷ったことは何?」

何度か上に書いた、コードサイズの大きなプルリクを受け取って「なにか有効なフィードバックをください!」とメッセージを貰うような状況。
そんな時に僕にとって、時間対効果の高い論点をサクッと導きやすい便利な言葉が「迷ったことは何?」という質問でした。
レビュイーに論点を提示してもらうことと原理は同じですが、この質問をすることでレビュイーが少し考えたあとに「そういえば」といって、実装時の悩みを伝えてくれることが結構あります。
レビュイーもプログラミングをしている時点では必死になっていることだと思います。冷静に振り返ってみると「あ、おかしなことをしていました」と自己解決して再実装に取り掛かってくれるようなこともありました。

名称未設定のノート (2)-9.jpg

3-3.人を攻撃しない

人に向かわず、コトに向かう。育成の観点がある場合は別かもしれませんが。基本的にはプロダクトの利得を意識してコードがどうあるべきかに注目すべきだと思います。
しかし当然、レビュイーからすると攻撃されるのは怖いですよね。レビュイーは自分のコードを提示する側ですし、場面的に否定されるように感じやすい状況です。
なかなかにいうは易し、行うは難しだなと思います。
相手が会社の同僚などであれば、普段の信頼関係の構築や、思いやりを伝えたり、プロ意識を確認しあったり。ここには正解はないですが、常に意識しておかないとなーと思います。

名称未設定のノート (2)-10.jpg


改めて整理し直すことで思いましたが、やっぱりレビューの前提となる論点設定、文脈構築、コミュニケーション方法の決定など、なかなかに複雑だなと思います。
もしそのようなことをやったことがないチームならば、ソースコードをレビューする前に、レビューすること自体をレビューしておくと、すこしチームメンバー同士が仲良くなるかもしれません😁
2020年も、皆様が幸せでありますように!

Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
Comments
No comments
Sign up for free and join this conversation.
If you already have a Qiita account
Why do not you register as a user and use Qiita more conveniently?
You need to log in to use this function. Qiita can be used more conveniently after logging in.
You seem to be reading articles frequently this month. Qiita can be used more conveniently after logging in.
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
ユーザーは見つかりませんでした