Slide 1

Slide 1 text

スタートアップ6年目の Review文化 @nakamura244

Slide 2

Slide 2 text

- Software Engineer (CyberAgent group) - @nakamura244 - Makuake - 創業エンジニア (from 2013) About me

Slide 3

Slide 3 text

知見を共有する事で新しい気づきを与える事ができ たり、皆さまと会話する中で自分も新たな気づきを得 て切磋琢磨していきたいと思いますので本日はよろし くお願い致します。 About

Slide 4

Slide 4 text

agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方

Slide 5

Slide 5 text

agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方

Slide 6

Slide 6 text

B --- 一言でReviewといっても色々ある 現チームでやっているReviewは主に下記の役 割がある 1. 意図しない不具合の防止 a. サービスの急成長による不具合はある程度許容 2. 保守性を向上させる 3. Reviewを通してmemberの成長 4. 本質的な開発へ

Slide 7

Slide 7 text

B --- フェーズ とにかく動くものをリリースして事業を回すこと が最優先事項だったフェーズから3年、5年先ま で使えるコードを書くフェーズへ移行 その中でのReview文化について話します 結局3年も経たないうちに書き換えるけど...

Slide 8

Slide 8 text

B --- 過渡期 6年以上も同じサービスやってると当然古い アーキテクチャとかは出てくる 新しい言語やアーキテクチャの採用はする 求められるエンジニアリングスペックが広くなる

Slide 9

Slide 9 text

agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方

Slide 10

Slide 10 text

ReviewとPull RequestはセットなのでReviewの Tipsだけではなく、Pull Requestも工夫が必要 !!

Slide 11

Slide 11 text

D tips Review Tips - 1 - 同じチーム内でReviewは完結させる事が基本 - ただし、チーム外だけどあの人は詳しいので一度見 てもらいたいとかはアリ - でもあまりに続くようならそもそもチーム編成に問題 がある可能性

Slide 12

Slide 12 text

D tips Review Tips - 2 - Reviewerにassignされてなくてもレビューしてし まおう! ❏ 目的はより良いコードにしてリリース ❏ Reviewer設定されてないとか関係ない ❏ PRだけではないissueもレビューしてしまう ❏ キャッチアップ力高めておくと障害発生時にヒーロー 的活躍ができる ❏ コードリーディングはいくらやっても損にはならない

Slide 13

Slide 13 text

D tips Review Tips - 2 - こんなイメージ

Slide 14

Slide 14 text

D tips Review Tips - 3 - Reviewとはcode reviewが全てでは無い。仕様 Reviewだったり、動作確認をするReviewだった り何個かある - 自分はXX言語はわからないからレビューができな い。とはならない。別視点を持てば貢献できる - 新しい言語やFWを入れた時などに有効 - アーキテクチャの変革期の時とか有効 - QAチームがいない時とかも有効

Slide 15

Slide 15 text

D tips Review Tips - 4 - 議論を呼びそうなPull Requestに対してはどち らかの手法を使う - 1. 関係者を集めて、はじめに議論してどういう方針で 実装して行くかを決めてからPull Requestを作る - 2. 実装しながらでないと分からない部分があるので 議論をする為のPull Requestを作る

Slide 16

Slide 16 text

D tips Review Tips - 4 - こういうやつです 中で紛糾しています

Slide 17

Slide 17 text

D tips Review Tips - 4 - 1の場合、 一度作った、書いたコードをmergeしたい人向 け。自分が書くコードがゴミになってcloseする 事にためらいがある人は向かない。

Slide 18

Slide 18 text

D tips Review Tips - 4 - 1を選択してても途中、設計に関わる指摘コメン トが付いたりする。この行為自体は問題はない - もう一度集合して設計方針の確認 - しかし、やっぱり違うと思ったら、事前の合意関係な く、異論を唱える事が重要 - 目的は滞りなく実装する事ではない

Slide 19

Slide 19 text

D --- Reviewの目的に立ち返る 1. 意図しない不具合の防止 a. サービスの急成長による不具合はある程度許容 2. 保守性を向上させる 3. Reviewを通してmemberの成長 4. 本質的な開発へ

Slide 20

Slide 20 text

D tips Review Tips - 4 - 2の場合、 自分が書くコードがゴミになってcloseする事に 何もためらいが無い人向け コードとしてはより良いコードになる傾向があ る。 コードは3回書き直せ文脈

Slide 21

Slide 21 text

D tips Review Tips - 4 - 2の場合、 こんな感じ

Slide 22

Slide 22 text

D tips Review Tips - 4 - どちらが正解という訳では無いけど、自分は2 のパターンを多用しています

Slide 23

Slide 23 text

D tips Review Tips - 5 - スコープの明記 ▸ このpull requestでのスコープを明記しないとあれも これも状態になる ▸ ただし、片手落ちになるようなものはスコープが間 違っている可能性ある。そこは見逃してはいけない

Slide 24

Slide 24 text

D tips Review Tips - 6 - レビューの観点を明記 - どう言ったところがポイントでこう実装してみたのでそ のあたりは良くみてもらいたい的な事を書いておく

Slide 25

Slide 25 text

D tips Review Tips - 6 - こんな感じ

Slide 26

Slide 26 text

D tips Review Tips - 7 - `XX以外はLGTM`コメント。 - Github上のキャッチボールを減らす効果 -> `XX以外 はLGTM`

Slide 27

Slide 27 text

D tips Review Tips - 8 - レビュアーの負担を軽減させる為に粒度は細か く。change fileは10程度が限度 ▸ Branchかbranchを派生させてレビューに出したりす る ▸ Branchからbranchを派生させては3回ぐらいが限度 ▹ Reviewファースルールと上手く合う ▸ Rebase ontoの多投

Slide 28

Slide 28 text

D --- Review Tips - 9 - LGTMの重さを理解する もし、自分がReviewして最終的にLGTMして、その後本 番環境で不具合が生じてしまったら、、、 レビュアーの責任も同じぐらいあるという自覚を (こう言うのを攻め立てる組織はないと思いますけど)

Slide 29

Slide 29 text

D --- Review Tips - 10 - レビュアーは権威的にならないように注意 NGをいうのならなるべく代替案を出してNGと言おう

Slide 30

Slide 30 text

agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方

Slide 31

Slide 31 text

E --- 出てきた課題 -1- Reviewが溜まる。溜まるとどんどん溜まる でも自分の作業はあるし、どこでReviewしたら 良いのか... 反応が無いとレビュイーもストレスを感じる

Slide 32

Slide 32 text

E --- 出てきた課題に対する対応 -1- - 1日の仕事始めはReviewから実施 - Reviewファースト。他の人の成果を重視 - N営業日以内に1st Reviewのコメントをつけ るルール - Review requestをこまめに見る

Slide 33

Slide 33 text

E --- 出てきた課題に対する対応 -1- Reviewファーストにするとどうしても自分の進捗 が以前よりも遅くなるという不満が出てくる

Slide 34

Slide 34 text

E --- 出てきた課題に対する対応 -1- - Reviewを通じて情報のキャッチアップ、潜在 的なバグの防止、コーディングスキル向上 なども立派なチームの成果としてあげる(経 営には見えづらいけど) - この状態が本来のスピードだと理解する - testコードを書かなければもうちょっと早くリリースでき るんだが、的な文脈の回答と同じ

Slide 35

Slide 35 text

E --- 出てきた課題 -2- いくらReviewファーストにしようが、振り返りで ルールを徹底しようとするが、できない人も出て くる

Slide 36

Slide 36 text

E --- 出てきた課題に対する対応 -2- 人のキャパシティはいきなりは上がらない 見守る

Slide 37

Slide 37 text

E --- 出てきた課題 -3- スコープ外だけど、思った事やコメントとして残 しておいた方が後々有益な情報になりそうな場 合がある。 スコープ外だから、コメントを差し控えると伝え る場を失う。 スコープ外だけどコメント残すとレビュイーとして はスコープ外なのでどう対応したら良いのか困 る

Slide 38

Slide 38 text

E --- 出てきた課題に対する対応 -3- 解決できていない...

Slide 39

Slide 39 text

agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方

Slide 40

Slide 40 text

F --- Reviewとは Review時間は事業からすると分かりにくい価値 ですが、エンジニアにとってはとっても価値のあ る作業です

Slide 41

Slide 41 text

F --- Reviewとは 優しさであり、建設的な議論の場であり、成長 の場です。 予定調和的な着地でなんの成長があるのか 常に謙虚に相手を尊重しながら自分の言いた い事は言うべき。

Slide 42

Slide 42 text

ご静聴ありがとう ございました