てくすた

ピクスタ株式会社のエンジニア・デザイナーがつづるよもやまテクニカルブログです

プルリクエストを作ってコードレビューをしてもらう際に気をつけていること

エンジニアの id:cheezenaan です。先月11月の話ですが、かのラーメン二郎目黒店から徒歩圏内という恵まれた立地へと引っ越してきました。これまで片道1時間弱かかってきた通勤時間が大幅に短縮できて、クオリティー・オブ・ライフ(QOL)の向上に強く貢献したと実感しています。ちなみに好きなコールは「ヤサイニンニクアブラカラメ」です。

さてピクスタ開発部では GitHub の Pull Request 機能によるコードレビューを導入しています。新規機能の開発から既存機能の改修まで、自分以外のメンバーによるレビューを通過した内容だけが master ブランチへマージされ、本番環境へリリースされます。

みなさんの会社や個人での開発でも、開発プロセスにコードレビューを導入している方が多いかと思いますが、今回は「私が他のメンバーへコードレビューを依頼する際に、どのような点に気をつけてプルリクエストを作成しているか」について、軽く話ができればと思います。

このエントリは、PIXTA Advent Calendar 2017 5日目の記事です。4日目は tokinaga による「未経験でウェブ業界に転職する前に準備したことと、その結果」でした。

こんなプルリクエストはいやだ(※個人の感想です)😫

コードレビューをしたことある方は、このようなプルリクエストを見たことがあるかもしれません。

  • diff が10,000行超え😇
  • No description provided 🤔
  • コミットメッセージから修正の意図が読み取れない😇

diff が10,000行超え😇

「あれもこれも…」と修正を加えていったら10,000行を超えてしまったのでしょう。diff が増えれば増えるほど、何をチェックすれば良いのかレビュアーは判断しづらくなります。

No description provided🤔

よほど自明で単純な修正であったり、プロジェクトに関わるメンバーがみな対象分野にくわしい場合は、Description を仔細に書かなくても問題ないかもしれません。しかしチーム開発の現場ではこのような都合のいい修正ばかりではありませんし、レビュアーとレビュイーとで業務知識やスキルに差がある場合が多いかと思います。「レビュアーは必ずしもレビュイーほど対象分野にくわしいわけではない」と留意する必要があります。

コミットメッセージから修正の意図が読み取れない😇

「なにを・なぜ修正したのか」という意図が読み取りづらいコミットメッセージは、レビュアーがプルリクエストを理解するにあたっての支障となります。たとえば fix の一言しかないコミットメッセージをレビュアーが見て、どう反応するでしょうか。ある機能に関する何かしらの仕様修正なのか、単に Rubocop や ESLint などフォーマッターの指摘修正なのか、想像できるでしょうか。また、コミットメッセージと実際の修正内容が一致しないコミットも、レビュアーに混乱させかねないので避けたいところです。

やっていること⚒

ピクスタ開発部にジョインしてから1年半、レビュアー・レビュイーとして数々のコードレビューを経験してきたなかで、「ここに気をつけると、コードレビューはもっとうまくいくかも…」と思うポイントをご紹介します。

  • 1つのプルリクエストで修正する diff はせいぜい200行まで✂️
  • Pull Request テンプレート機能を利用して必要な情報を書き込む✍️
  • コミットの粒度はストーリーを実現するためのタスク単位に揃える📏

1つのプルリクエストで修正する diff はせいぜい200行まで✂️

diff が増える要因として、プルリクエストの中に大量のストーリーが含まれているために修正規模が大きくなっていることが主に考えられます。ここでいう「ストーリー」とは 「○○(who)が○○できる(can what)」 という形式で表現できるものを指します。

まずはストーリーを細かい粒度に分割していきます。できれば各スプリント前のプランニングの段階でできているとベストです。実装を進めていく中で diff が数百行単位で増えてきた場合は途中でも作業を一時中断し、ほどよい粒度にブランチを分割してから作業を再開します。

個人的には、1つのプルリクエストの diff が200行以内に収まると、レビュアー・レビュイーともに負担を覚えることなくコードレビューを進められると感じています。大きくなったストーリーを分割するコツは『アジャイルな見積りと計画づくり』が非常に参考になりました。

アジャイルな見積りと計画づくり ~価値あるソフトウェアを育てる概念と技法~

アジャイルな見積りと計画づくり ~価値あるソフトウェアを育てる概念と技法~

  • 作者: Mike Cohn,マイクコーン,安井力,角谷信太郎
  • 出版社/メーカー: 毎日コミュニケーションズ
  • 発売日: 2009/01/29
  • メディア: 単行本(ソフトカバー)
  • 購入: 74人 クリック: 764回
  • この商品を含むブログ (226件) を見る

Pull Request テンプレート機能を利用してレビューに必要な情報を書き込む✍️

ピクスタでは他のメンバーへコードレビューを依頼する前に、GitHub が公式に提供する Pull Request テンプレート機能を利用した Description 記載ガイドラインに沿って、レビューに必要な項目を埋めるようにしています。

f:id:cheezenaan:20171122193245p:plain

テンプレートでは主に以下の項目を記入するようになっています。

  • 概要・技術的変更点: そもそものストーリーの説明や、ストーリーを実現するために行った変更点。文字だけで伝わりにくい場合は適宜スクリーンショットも添えます
  • 見どころ: とくに見てほしいポイント、あるいは見なくてもよいポイントを書きます
  • レビュー完了希望日: そのプルリクエストの緊急度を伝えます。不具合対応なので今すぐ見ないとマズそうだとか、軽微な改修なのである程度日を置いても大丈夫だ…と伝わります
  • テスト項目: 自動テストやブラウザ上での目視・手動操作による確認項目を書きます

テンプレート機能のくわしい使い方は、GitHub 公式のリンクを参照ください。

Creating a pull request template for your repository - User Documentation

コミットの粒度はストーリーを実現するためのタスク単位に揃える📏

ひとつのコミットでは、ひとつの変更だけを扱うようにします。具体的には以下のことを意識するとうまくいく場合が多いです。

  • ライブラリの導入や既存コードのリファクタリング、機能の追加・改修でおおまかに作業を分ける
  • ESLint や Rubocop などチェックツールの指摘修正はそれとわかるように Lint 等のメッセージをつける

たとえば「既存のページに試聴プレイヤーを組み込む」場合で考えてみると、このように分割できるのではないでしょうか。

  • ライブラリの導入(yarn installbundle install の結果だけをコミット)
  • ページへの組み込み
    • 既存コードのリファクタリング
    • 機能追加A
    • 機能追加B
    • Rubocop や ESLint などのフォーマッターによる指摘対応
    • …など、実際にはもう少し細かく分割することもあります

f:id:cheezenaan:20171127183828p:plain

PIXTA のサービス開発ではタスク管理に Backlog を使用しており、実際のコミットメッセージには [Backlog のチケット番号] (作業内容) という形式で記録しています。チケット番号と作業内容をコミットメッセージで紐づけることで、コードの変更意図を追いかける際に「コードに対して git blame する → 該当するチケットを Backlog から探す → 施策の背景やあらましをつかむ」という一連の動作がスムーズにできます。

適切な粒度に作業を分割できていれば、コミットメッセージもおのずと修正内容に沿ったシンプルな内容に決まるため、読み手にも意図が伝わりやすくなります。それでもなお「何をメッセージに残すか?」と悩んだときは、偉大なるプログラマである @t-wada 氏のツイートを思い出すといいかもしれません。

まとめ

「私がコードレビューでプルリクエストを作成する際に気をつけていること」をつらつらと挙げてみましたが、これらの根底にあるのは「チーム開発をともに進める他のメンバーへの細かい気配り・思いやり」なのかなと思いました。

コードレビューはレビュアー・レビュイーがともにコードと向き合って、よりよいプロダクトをつくるための習慣です。小さな工夫や心づかいを積み上げて、このすばらしい習慣をよりよく・長く続けていきたいものです。

そしてまた、次のアドベントカレンダーがはじまるのです

6日目の記事は、私と同じ2016年8月入社の kenue が担当します。ご期待ください!

エンジニア募集中!

ピクスタでは、細かい気配りをしながらチーム開発を進めるのが得意なエンジニアを募集しています。

未経験でウェブ業界に転職する前に準備したことと、その結果

この記事は、PIXTA Advent Calendar 2017 4 日目の記事です。

2017年8月にピクスタに入社した、エンジニアの tokinaga です。 本記事は、ウェブ業界に未経験ながらも転職した私が、事前準備としてどのような学習を行ったのか、そしてその結果どうなったのかをまとめたものです。 私と同じようにウェブ業界への進出を目論むエンジニアの方々にとって、少しでも参考になれば幸いです。

ピクスタ入社前の私

背景

前職を 2017 年 6 月末に退職しました。 ピクスタの内定をもらったときに「ハネムーンとか行きたいんで 8 月からの出社でいいですか」と古俣さん(社長)に伝えたら OK されたので、1 ヶ月間夢の NEET 期間を享受できることに。 せっかくなのでガッツリ勉強することにしました。
(ハネムーンは6月末の有休消化期間に行きました)

前職退職直後のレベル感

前職退職後の私は以下のような感じでした。

  • 文系大学卒
  • 制御系の企業でソフトウェアエンジニア経験あり(3 年間)。
    • 使用言語は主に C 。
    • 基本は Windows マシンから 共有 Linux マシンに ssh 接続して開発するスタイル。
  • DB って何? curl って何?

正直ウェブの知識はさっぱりで、ポテンシャル採用に賭ける!という意気込みで転職活動をしていました😇

ピクスタ入社までの 1 ヶ月間

やったこと

Backend 周り

ピクスタでは Rails を使っているので、Rails の勉強には最も時間を割きました。

  • Ruby on Rails Tutorial (以後 Tutorial と記載)

    Rails を学ぶものなら誰もが 1 度は耳にする、おなじみの Tutorial です。 圧倒的な分量なので完走するには気合と根性が必要ですが、乗り越えると実務レベルの Rails 力が身につくと思います。

  • Ruby on Rails 5アプリケーションプログラミング

    Ruby on Rails 5アプリケーションプログラミング

    評判が良かったので購入。分厚い本なので読み切れるか不安でしたが、Tutorial に比べたらスラスラ読めてサクサク進むので、非常に楽しめます。 Tutorial では実践的に学ぶことに主眼が置かれていますが、こちらは体系的に丁寧に説明してくれるので、断片的だった知識が次々と整理されていく喜びがあります。 Tutorial をやった後に読んだからこそ、ここまで効果を実感できたのではないかなという感想を持ちました。

  • Ruby on Rails Tutorial (2 周目)

    1 周目と比べてどれくらい理解が進んでいるかを試すために、2 周目をしました。正直しんどかった記憶しかないので、無理してやらなくても良かったかなという気持ちがあります。 後から調べたところ、Tutorial は PDF 換算だと 808 ページにもなることがわかり、衝撃を受けました。

Frontend 周り

  • 改訂新版JavaScript本格入門 ~モダンスタイルによる基礎から現場での応用まで

    改訂新版JavaScript本格入門 ~モダンスタイルによる基礎から現場での応用まで

    JavaScript 恐怖症を克服するために読破。困った時にググるだけでは一生気がつかなかっただろうなというような仕様がきっちり体系的に書かれていたので、非常に助かりました。

  • デザイン系

    以下の 4 つをドットインストールで学びました。

    一般的にはデザイナーが担当することになる領域なのですが、自分でプロダクトを作る際には避けては通れないため、取り組みました。

DB

  • スッキリわかる SQL 入門 ドリル215問付き!

    スッキリわかる SQL 入門 ドリル215問付き! (スッキリシリーズ)

    DB を一切使ったことがなかったため、基礎から力をつけるために取り組みました。かなりわかりやすく、容易に理解できました。

開発技法

  • アジャイルサムライ

    アジャイルサムライ−達人開発者への道−

    ピクスタではアジャイル開発を導入していると聞いていたので、読んでおきました。「アジャイルってこんなものでしょ?」とイメージしていたものとの乖離が大きかったので、アジャイルというものをイメージだけで捉えている方々は一度読んでみると良いかと思います。

その他

寺社仏閣マップを学習目的で、友人と開発しました。 ログイン機能もないシンプルな静的サイトです。 書籍やウェブ教材で勉強しているだけだとダレてきてしまいますが、友人との開発があるとワクワク感が生まれ、日々にハリが生まれた気がします。

やらなかったこと

以下は、学習計画を立てた際に意識はしていたものの、見送られたものです。

  • ネットワーク周りの学習(HTTP、TCP/IP の知識拡充)
  • AWS でインスタンスを立ち上げてサービスをデプロイしてみる
  • docker 環境を自分で構築してみる

ネットワーク周りの学習は、ひとまず Rails が使えるようになってウェブサービスのイメージがついてから取り組んだ方が身につくのが早いと感じていたので、優先度を下げていました。時間があれば取り組む予定でしたが、間に合いませんでした😇
AWS/docker といったインフラ周りは、入社後ピクスタでの実際の運用を見て参考にしてから学んだ方が効率が良いかな?と感じ、スルーしました。

ピクスタ入社後の私

入社後はすぐにサービスの開発にアサインされました。未経験だったので不安はありましたが、1 ヶ月の間で学んだことがそのまま活かせるケースが想像以上に多いということに気づき、非常に安心しました。
「なんだ、意外とやれるじゃん」というのが正直な感想でした(もちろん働いているうちに、いかに自分が未熟かを思い知ることにはなりますが)。
また、一通り広い分野を学んでいたおかげで、会話に専門用語的なものが出てきた時にきちんと理解できるようになっていたのは良かったのかなと思います。用語を理解できると円滑なコミュニケーションが取れるため、スムーズに業務に取り組めます。
また、意外に有効だったのはデザイン周りの学習です。仕事ではデザイナーの方と一緒に働く機会が多いのですが、デザイナーの方がどのような仕事をしているかの大枠を捉えておくとコミュニケーションが円滑になり、仕事を進めやすいです。

特に役に立ったこと

特に役に立ったのは、やはり Ruby on Rails Tutorial でした。このおかげで入社後すぐに作業に取りかかることができましたし、Tutorial をきちんとこなしたこと自体がそれだけで少なからず自信をもたらします。
Tutorial だけこなせば、Rails で開発している会社である程度即戦力として働けると言っても過言ではない気がします 1

反省点

JavaScript と SQL については、もう少しがっつり学ぶ機会があったら良かったなという印象を受けました。どちらも本を読んだだけで実践的に使う機会が少なかったという共通点があります。そのような状態だと、身につく前にすぐ忘れてしまいます(実際ほとんど忘れました)。ソースコードやクエリ文を、「読んだらかろうじて理解できる」レベルから、「ある程度スラスラ書ける」レベルまでは一気に持っていくべきでした。

まとめ

  • ウェブ業界未経験でも、ある程度準備をすればきちんと仕事をこなせるようになります。
    • なんとなく一歩目を踏み出せない方は、とりあえず Ruby on Rails Tutorial に取り掛かりましょう!
  • まとまった期間自分のやりたいことにじっくり取り組める機会なんてそうそうありません。もしあなたに転職する予定があるのならば、是非束の間の NEET 期間を満喫することを強くオススメします。その期間収入は失うことになりますが、それをはるかに上回る成長と幸福感があなたを待っていると思います。

エンジニア募集中!

ピクスタでは、意欲に燃えるウェブ業界未経験のエンジニアを募集しています。


  1. 個人の感想であり、効果には個人差があります