スタートアップ6年目のレビュー文化.pdf
by
tsuyoshi nakamura
Link
Embed
Share
Beginning
This slide
Copy link URL
Copy link URL
Copy iframe embed code
Copy iframe embed code
Copy javascript embed code
Copy javascript embed code
Share
Tweet
Share
Tweet
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
ご静聴ありがとう ございました