スタートアップ6年目のレビュー文化.pdf

Transcript

1. スタートアップ6年目の Review文化 @nakamura244
2. - Software Engineer (CyberAgent group) - @nakamura244 - Makuake - 創業エンジニア (from 2013) About me
3. 知見を共有する事で新しい気づきを与える事ができ たり、皆さまと会話する中で自分も新たな気づきを得 て切磋琢磨していきたいと思いますので本日はよろし くお願い致します。 About
4. agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方
5. agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方
6. B --- 一言でReviewといっても色々ある 現チームでやっているReviewは主に下記の役 割がある 1. 意図しない不具合の防止 a. サービスの急成長による不具合はある程度許容 2. 保守性を向上させる 3. Reviewを通してmemberの成長 4. 本質的な開発へ
7. B --- フェーズ とにかく動くものをリリースして事業を回すこと が最優先事項だったフェーズから3年、5年先ま で使えるコードを書くフェーズへ移行 その中でのReview文化について話します 結局3年も経たないうちに書き換えるけど...
8. B --- 過渡期 6年以上も同じサービスやってると当然古い アーキテクチャとかは出てくる 新しい言語やアーキテクチャの採用はする 求められるエンジニアリングスペックが広くなる
9. agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方
10. ReviewとPull RequestはセットなのでReviewの Tipsだけではなく、Pull Requestも工夫が必要 !!
11. D tips Review Tips - 1 - 同じチーム内でReviewは完結させる事が基本 - ただし、チーム外だけどあの人は詳しいので一度見 てもらいたいとかはアリ - でもあまりに続くようならそもそもチーム編成に問題 がある可能性
12. D tips Review Tips - 2 - Reviewerにassignされてなくてもレビューしてし まおう！ ❏ 目的はより良いコードにしてリリース ❏ Reviewer設定されてないとか関係ない ❏ PRだけではないissueもレビューしてしまう ❏ キャッチアップ力高めておくと障害発生時にヒーロー 的活躍ができる ❏ コードリーディングはいくらやっても損にはならない
13. D tips Review Tips - 2 - こんなイメージ
14. D tips Review Tips - 3 - Reviewとはcode reviewが全てでは無い。仕様 Reviewだったり、動作確認をするReviewだった り何個かある - 自分はXX言語はわからないからレビューができな い。とはならない。別視点を持てば貢献できる - 新しい言語やFWを入れた時などに有効 - アーキテクチャの変革期の時とか有効 - QAチームがいない時とかも有効
15. D tips Review Tips - 4 - 議論を呼びそうなPull Requestに対してはどち らかの手法を使う - 1. 関係者を集めて、はじめに議論してどういう方針で 実装して行くかを決めてからPull Requestを作る - 2. 実装しながらでないと分からない部分があるので 議論をする為のPull Requestを作る
16. D tips Review Tips - 4 - こういうやつです 中で紛糾しています
17. D tips Review Tips - 4 - 1の場合、 一度作った、書いたコードをmergeしたい人向 け。自分が書くコードがゴミになってcloseする 事にためらいがある人は向かない。
18. D tips Review Tips - 4 - 1を選択してても途中、設計に関わる指摘コメン トが付いたりする。この行為自体は問題はない - もう一度集合して設計方針の確認 - しかし、やっぱり違うと思ったら、事前の合意関係な く、異論を唱える事が重要 - 目的は滞りなく実装する事ではない
19. D --- Reviewの目的に立ち返る 1. 意図しない不具合の防止 a. サービスの急成長による不具合はある程度許容 2. 保守性を向上させる 3. Reviewを通してmemberの成長 4. 本質的な開発へ
20. D tips Review Tips - 4 - 2の場合、 自分が書くコードがゴミになってcloseする事に 何もためらいが無い人向け コードとしてはより良いコードになる傾向があ る。 コードは3回書き直せ文脈
21. D tips Review Tips - 4 - 2の場合、 こんな感じ
22. D tips Review Tips - 4 - どちらが正解という訳では無いけど、自分は2 のパターンを多用しています
23. D tips Review Tips - 5 - スコープの明記 ▸ このpull requestでのスコープを明記しないとあれも これも状態になる ▸ ただし、片手落ちになるようなものはスコープが間 違っている可能性ある。そこは見逃してはいけない
24. D tips Review Tips - 6 - レビューの観点を明記 - どう言ったところがポイントでこう実装してみたのでそ のあたりは良くみてもらいたい的な事を書いておく
25. D tips Review Tips - 6 - こんな感じ
26. D tips Review Tips - 7 - `XX以外はLGTM`コメント。 - Github上のキャッチボールを減らす効果 -> `XX以外 はLGTM`
27. D tips Review Tips - 8 - レビュアーの負担を軽減させる為に粒度は細か く。change fileは10程度が限度 ▸ Branchかbranchを派生させてレビューに出したりす る ▸ Branchからbranchを派生させては3回ぐらいが限度 ▹ Reviewファースルールと上手く合う ▸ Rebase ontoの多投
28. D --- Review Tips - 9 - LGTMの重さを理解する もし、自分がReviewして最終的にLGTMして、その後本 番環境で不具合が生じてしまったら、、、 レビュアーの責任も同じぐらいあるという自覚を (こう言うのを攻め立てる組織はないと思いますけど)
29. D --- Review Tips - 10 - レビュアーは権威的にならないように注意 NGをいうのならなるべく代替案を出してNGと言おう
30. agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方
31. E --- 出てきた課題 -1- Reviewが溜まる。溜まるとどんどん溜まる でも自分の作業はあるし、どこでReviewしたら 良いのか... 反応が無いとレビュイーもストレスを感じる
32. E --- 出てきた課題に対する対応 -1- - 1日の仕事始めはReviewから実施 - Reviewファースト。他の人の成果を重視 - N営業日以内に1st Reviewのコメントをつけ るルール - Review requestをこまめに見る
33. E --- 出てきた課題に対する対応 -1- Reviewファーストにするとどうしても自分の進捗 が以前よりも遅くなるという不満が出てくる
34. E --- 出てきた課題に対する対応 -1- - Reviewを通じて情報のキャッチアップ、潜在 的なバグの防止、コーディングスキル向上 なども立派なチームの成果としてあげる(経 営には見えづらいけど) - この状態が本来のスピードだと理解する - testコードを書かなければもうちょっと早くリリースでき るんだが、的な文脈の回答と同じ
35. E --- 出てきた課題 -2- いくらReviewファーストにしようが、振り返りで ルールを徹底しようとするが、できない人も出て くる
36. E --- 出てきた課題に対する対応 -2- 人のキャパシティはいきなりは上がらない 見守る
37. E --- 出てきた課題 -3- スコープ外だけど、思った事やコメントとして残 しておいた方が後々有益な情報になりそうな場 合がある。 スコープ外だから、コメントを差し控えると伝え る場を失う。 スコープ外だけどコメント残すとレビュイーとして はスコープ外なのでどう対応したら良いのか困 る
38. E --- 出てきた課題に対する対応 -3- 解決できていない...
39. agenda ▸ 本資料の前提情報 ▸ makuake的ReviewのTips ▸ 運用の中で出てきた課題 ▸ Reviewについての考え方
40. F --- Reviewとは Review時間は事業からすると分かりにくい価値 ですが、エンジニアにとってはとっても価値のあ る作業です
41. F --- Reviewとは 優しさであり、建設的な議論の場であり、成長 の場です。 予定調和的な着地でなんの成長があるのか 常に謙虚に相手を尊重しながら自分の言いた い事は言うべき。
42. ご静聴ありがとう ございました