スタートアップ6年目のレビュー文化.pdf

by tsuyoshi nakamura

Embed

Start on current slide

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

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