見出し画像

心地良い開発環境をエンジニアに提供するための ミツモア式コードレビューとは

各社が採用する共通の「良いとされているテンプレートのレビュールール」は存在しつつも、やはり会社によって、メンバーによってコードレビューの体験は異なります。ミツモアのコードレビューの実態を、ミツモアに初期から関わる“開発コア5”の一人である坂本竜に、経験も含めて語ってもらいました。

画像1

坂本竜(@ryusaka) プロダクト部 開発3グループ
早稲田大学で情報工学を学び、インターンを経て2019年にミツモアに新卒第一号として入社。Node.js, Reactを用いてフロントエンド、バックエンド共に開発。社内にて既存プロジェクトへのTypescriptの環境構築、普及を行なう。
https://github.com/ryusaka
https://qiita.com/ryusaka

理想は全員がレビューできること

ーミツモアのコードレビュー体制についてまずは基本的なことを教えてください。

坂本:GithubのPull Request機能(以下PR)を使っています。ミツモアのレビューガイドラインがあって、レビューを始めたばかりの人にはそれを読んでレビューしてもらうようにしています。ガイドラインには、基本的なコーディングの作法や、その他気をつけることが書いてあります。

ー誰がレビューするかなど決まっていますか?

坂本:明確なルールはないですが、コードレビューはチームリードがする場合が多いです。基本的に主にその部分の開発に関わった人(コードオーナー)が見ます。チームリードは、開発した人もしくは把握している人の場合が多いからです。また、チームをまたいですることもあります。

ーチームをまたいでという会社も珍しいような気がします。

坂本:会社によっては「チームが違う=プログラムが違う」のですが、ミツモアの場合は、新規事業のプロダクト以外は、見積もりプラットフォーム「ミツモア」というプロダクト1個を4チームで開発しているという状況です。そうなってくると、同じプロダクトのあの部分、この部分、みたいな感じを担当しているので、どうしてもまたいでいる部分も出てきます。それから、コミュニケーションを促進するという目的で半年に1回チームのメンバーを入れ替えてチームシャッフルをしています。そうしたこともあって、2ヶ月前このコードを書いた人はもう違うチームにいるという場合もあるので、チームをまたいでのレビューはよくありますね。

ーあまりガッチリと組織立ってレビューするイメージではなさそうですね。

坂本:そうですね。レビューする人はこの人、されるのはこの人、と決まっていません。レビューできる人、変更箇所に一番詳しい人がレビューをしたり、レビュイー/レビュアーが役職じゃなくて、その時のやるべき人になっています。ですので、みんながレビューする機会があります。レビューすると、自分が書くコードへの俯瞰力も上がります。できるだけレビューできる人を増やそうとしているので、そういった経験は積めると思います。みんながコードレビューをできるようになるのが理想と考えています。

明確なレビュールールの必要性が芽生えた頃


ー少しパーソナルなことを伺います。これまでで、坂本さんが印象に残っているコードレビューを教えてください。

坂本:ユージーンと喧嘩になったコードレビューですね。

ーえ、それはいつぐらいですか?

坂本:ミツモアの初期で、ほぼ5人で開発していた時期です。

ーまだエンジニアが坂本さんと、ユージーンさん、CTO柄澤さん、寺井さん、白柳さんの5人しかいなかった時期ですね。

坂本:はい。ちょうど当時、ミツモアというプロダクトがどんどん機能を足されて、コード量が増えている時期でした。当時は5人しかいないのでコミュニケーションしやすかったですし、10しかコードがない時からいたので、100になる過程を5人が知っている状態。100あってもみんなが大体わかるという状態でした。ただ、今後採用拡大して、100から始める人もいる訳だから、メンテしやすいコードを書いていかないといけない、という意識が5人の中で出始めた時期だったんです。

ー多人数で開発しやすいコードへ移行しないといけない時期、ということですね。

坂本:もう少しその時の背景をお話しすると、ミツモアの登録プロ専用アプリ「ミツモアpro」を作ってる時にリリースが遅れていました。「早くリリースしたいユージーン」VS「そこはちゃんとしないとだめでしょという僕」という構図でした。当時は今より英語ももっとできなかったですし、うまくコミュニケーションできていなかった部分もありました。他に白柳さんからきていたPRは僕は通していたんですよね。ユージーンからすると「僕のPRはだめなのに、そっちのPRは良いって、ダブルスタンダードじゃないか」と怒られました。それでお互いにストレスみたいになってしまいました。実際こっちはこうでみたいな明確な基準は持たずに、若干本当にダブルスタンダードみたいな部分も、僕の中にありました。なぜかと言うと、当時はルールがなかったけれど、みんなそれぞれがクオリティ高いコードが書けるから「レビューもそれぞれの感覚でやっても問題ない」という甘えがみんなあったのでしょうね。

ー5人以外のエンジニアを想定して、ルール整備が必要になってきたのですね。

坂本:ルールを決めると何がいいかというと、議論の時間が省けること。言い争いみたいなものは時間がもったいないですし、最終的にはメンバー間の喧嘩みたいになってしまうので、お互いに気持ちの良いものじゃない。書き方が何パターンもあるものは感覚的な部分もあって、“きのこ/たけのこ論争”(註=「きのこの山」と「たけのこの里」のどちらが美味しいといった、好みの問題の趣旨の論争)みたいになったりする。その辺はちゃんとルールを決めて、「お互いが決めたルールを一緒に守ろう。そうしなきゃいけない理由はルールだから」とルールのせいにできる。ルールが決めてくれれてば、その議論の時間を省ける。時間の効率も良くなりますし、後から入った人も、このルールに従って書けばミツモア的いいコードが書ける。もしルールを作っていなければ、新しい人が入る度に説明しなければならない。そう考えるとルールがあることは効率がいい。

ールール作りが快適な開発環境にも繋がるのですね。ちなみに、レビューツールなどは導入されてますか?

坂本:ルールを仕組みでチェックするためのツールとしてESLintを入れています。いわゆるリンターと言われる類のツールですけど、JavaScriptを静的に解析して、「この書き方はミツモアで決めたルールだとだめ」みたいな感じの指摘をしてくれます。このようにレビューを機械にやらせたりルールを決めたりすることで、みんなが心地よく開発できる環境を整えて、効率を上げていくことが、大事なのかと思います。

TypeScriptに移行しチームで開発しやすいコードへ

ー2021年2月現在、ミツモアの開発部で働いているエンジニアやデザイナーは34人(註:取材時)だそうですが、大人数で開発しやすいTypeScriptへ移行したそうですね。

坂本:移行自体は少し前に終わっていましたが、移行した後のTypeScriptの旨味を出すための追加の作業的なもの、TypeScriptによる付加価値を最大化するためのタスクを今期やっていたということですね。

ー今期というのは?

坂本:ミツモアでは、3ヶ月ごとに目標を決め、達成していくための数字に分解していき、その数字が達成できたら、最初に置いた目標も達成できる、というOKRをやっています。12月から2月が今回のターム(註:取材時)になっていて、エンジニアチームの目標は「新しいエンジニアがたくさん増えているので、新しく入った人が理解しやすいコードにしよう」でした。

OKRでは、それに対して数値目標を置くのですが、今回は「① javascriptからtypescriptへ移行して型がまだない」「②レスポンスデータの型定義 & それをやりやすくするレールの整備」「③FE(フロントエンド)の状態管理を型に強くて書き方を統一できる新しいフレームワークへの移行」という3項目に分けて行いました。

ーそれをエンジニア34人で行ったのですか?

坂本:デザイナーさんやデータチームは別の目標を置いてもらっていて、いわゆる開発をしている5チームのエンジニアたちで行いましたね。「10%ルール」と言って、全体の開発の10%という意味ですが、OKRの内容はプロダクト開発に必須な項目ではない。でもやらないと、将来的に開発の効率が落ちたり、事故が起きたりするのでやっておく、いわゆるリファクタリングといわれている活動です。それを業務の10%ぐらいの時間を使ってやろうというルールを開発チームでは敷いています。そして、このタスクは、普段の業務とは別に開発チームでやっています。それぞれのやりたい項目のところに属してもらって、横断的なチームを一時的に作り、そこでどのファイルをやるか決めて、お互いにレビューをして、やっていたという感じですね。

ーそれでは3項目について掘り下げてご説明お願いします。まずは①について。

坂本:TypeScriptはJavaScriptの上位互換と言われていますが、変換しただけだとメリットはほぼありません。TypeScriptにしかない「型」という旨味があるんですけど、その旨味を使えるように、必要な情報を足したり、ちょっと書き換えたりする作業ですね。「型」とはなんぞやという話ですが、JavaScriptは型がない言語で、例えば「タイヤ」という型がある。タイヤ型には自転車や車のタイヤが入れられます。でも、JavaScriptだったら、タイヤ型にスマートホンをつけることができてしまう。走る訳がないのに、仕組み上できてしまう。一方、TypeScriptは「車用のタイヤをはめてください」というコードが書けて、それ以外のものをはめようとするとエラーになるんですよ。普通の人なら「車のタイヤ」と言えば間違えないですが、例えば原始時代の人はタイヤが何かわからないから間違う可能性がある。これをプロダクトで例えると、ミツモアを作ってきた人にはそこに何をあてはめるかわかるけど、昨日入社して、ここ開発してくださいと言われた時にTypeScriptだと型で「車用のタイヤ」入れてねと書いてあるので、初見でもわかる。というのが、TypeScriptの良いところです。間違ってはめる事故も検知できるので、事故が起きなくて安全というのが、主にあげられるメリットですね。

ー②についてお願いします。

坂本:FE(フロントエンド)とBE(バックエンド)の間で、基本的に型情報が消えてしまうので、それぞれで人間が型を書き直さないといけないんですよ。毎回BEにデータを足したら、FEにもデータを足してやらないと、コードを書けないしエラーになってしまう。すごく簡単に言うとめんどくさいんですよね。どっちかを忘れたらバグの原因になる。そういうのをフレームワークが決めた書き方に則って書いていくと、例えばBEのコードを書いたら、自動でFE用のコードも作ってくれるツールがあったりして、そうすると通信の間で、型が消えることを気にしなくて良くなるんですね。そのツールを使いやすくするための型(サーバーが返してくれるレスポンスデータの型)を作って、かつフレームワークが決めたレールに則って型の情報を書くと、最終的にFEの型を勝手に作ってくれる。開発効率も上がるし、バグも減らせます。

ー最後に③についてお願いします。
 
坂本:いくら型があっても、結局書くのは人間なので、書こうと思ったらどんな書き方もできてしまいます。別にTypeScript的にオッケーだし、その書き方もこの書き方もメリット/デメリットあって、どっちでも良いよね、という書き方はあるので、レビューもどっちでも通してしまいます。そうすると同じような処理をいろんなパターンでしているだけなのに、書き方が違って、違うコードに見える。修正したい時に書き方に合わせて変えないといけないから、機械的に直したりできなくなる。そうすると開発効率が落ちる。それを防ぐため強制的にこの書き方をしてくださいという枠組みを提供してくれるものがフレームワーク。頭の良い人が議論して結論づけた書き方になっているので、例えば汎用性が高かったり、いろんなものに対しても変更に強い書き方ができるので、質の良いコードを簡単に誰でも書けるようになる、というのがフレームワークの良いところ。ミツモアでも一部導入されていましたが、ちょっと変更する場所が多くて導入し切れてなかった。それに全体的に移行してしまいましょう!というのをやりました。

ーちなみにフレームワークの名前を教えてもらっていいですか?

坂本:NestJS、OpenAPI、Swagger、typescript-fsaです。以上①②③を全部組み合わせると、TypeScriptのパワーをフルにより強く活用して、効率よく安全な開発ができる、というような感じです。

積極的に議論し、試し、良いところは褒める文化


ーリファクタリングによって新人の方にも理解しやすいコードになっているのですね。レビュールールも整ってきているようですが、そのほかにミツモアのエンジニア同士で意識していることはありますか?

坂本:最近特に共通意識を持つようにしているのは、ドキュメント、コメントを残すということです。何をする関数なのか、なぜこの書き方をしているのか等、JSDocなどコード内のコメントを残すようにしています。

ーなるほど。他にミツモアのコードレビューの特徴みたいなものはありますか?

坂本:必要に応じていろんな人を巻き込むことですね。5、6人が議論しまくっているPRもたまにあります。

ーそれはどういったことで議論するのですか?

坂本:大体は新しいフレームワークとかツールを大々的に入れようとなった時ですね。ミツモアのソースコードの管理のフォルダ構造をちょっと変えるとか、全体に横断的に影響が出る系のことは、割と、ああだこうだ議論をすべきだと思いますし、なることが多いです。そういうPRは、みんな見にきて、「僕はこう思う」とか、「ここはこうじゃないか」みたいなのを議論したりはしてますね。

ー収集つかなくなったりしませんか?

坂本:します(笑)。そういう新しいツールは、議論の俎上に載せて、メリット/デメリットをみんなで話した上で、結構やってみようとなることは多いです。特にプロダクト本体に関係ないコードの自動化ツールなどはジャンジャン試してみてますね。特にその辺は、どうしても英語が母国語のエンジニアがいるのは強みだと思っていて、話題になったものが日本語の記事になるのは、少なくとも数日遅れるんですよね。下手すると一生流行らないパターンもあって、ユージーンがシリコンバレーで最近話題のツールなどを持ってきてくれるのは強みですね。僕たちが翻訳された記事を読んで盛り上がってると、ユージーンは「3日前に見たな」と毎回思うそうです(笑)。TypeScriptから違う言語に変えるとか、そう言うのは簡単にできないし、よっぽどのことがない限りやらないんですけど、周りを取り巻くツール系は、なんでも試してみてますね。

ーミツモアらしいエピソードですね。

坂本:あとは、必要ならちゃんとchange requestする。たとえ急いでいても、将来大きな負債になりそうな部分に関しては、ちゃんと一度返します。一定サイズ以上のPRで、あまり一発で通ることはないかもしれませんね。

ーそこは新人の方は心に留めておいて良いかもしれませんね。まだ外国籍のエンジニアが多いミツモアならではのこともありそうですね?

坂本:褒めると言うか「この書き方好きだから、僕も今後やろう」とか、「こんなのあったんだ!」とか書くようにしていますね。その辺はやっぱり外国人の方が褒めるのが上手いなと思いますね。自分がやったことを自分から発信して、それに「いいね」がついたりしているのも良く見ます。お互いが気持ちよくなるようなコミュニケーションが上手いな〜と思いますね。会社全体としても、「マルチプライヤー(リズ・ワイズマン他著の全米ベストセラー「メンバーの才能を開花させる人」のこと)」というのが流行っているのですが、良いことした人はみんなの前で紹介して、皆で「ありがとう」と言ったり、そういうところがミツモアの文化としてあるかなと思いますね。

ーエンジニアの方だけでなく会社全体で「いいね!」文化が浸透しているのですね。

(取材・執筆 吉田千枝子 字と図)

======================================

現在、ミツモアは事業拡大を進めており、エンジニア・デザイナー・PdMをはじめ多くの職種で積極採用中です。
Wantedlyにて募集しているので、カジュアルに面談に来てみませんか?エンジニアチームの詳しいご紹介は「ミツモア エンジニア向け会社説明資料 / about meetsmore for engineers」を公開しております。



この記事が気に入ったらサポートをしてみませんか?