hacktk's blog

日記です

こんな風に開発してました

コードレビュー

目的 (将来の展望も含め)

  • 品質を上げる (バグなどの早期発見、readableなcodeを目指す)
  • 生産性を上げる (他人のコードを見る機会を作る → ベストプラクティスの共有)
  • 属人性の排除 (仕様を共有することでプロジェクトを1人しかメンテできない状態をなくす)
  • WIPプルリクによる、作業や仕様の共有・相談
    • WIPプルリクとは、作業を始める前からプルリクを出すこと
    • 作業の内容に対してレビューを受けることができる
    • 作業の進捗に合わせてレビューを受けることができる

レビュアーが意識するとよいこと

  • レビュー観点 (仕様、速度、可読性、変更容易性など)
  • やり過ぎない、重箱の隅をつつき過ぎない
  • 良い変更には LGTM する (言葉に出す、伝える)
  • 人格批判はNG
  • レビュー待ちがあまり多くならないよう、早めにレビューする(できれば当日中)
  • コメントはcommitではなくプルリクの Files changed でする (rebaseなどでcommitハッシュが変わっても消えない)
  • 作業レビューは、やり方に問題ないかなどを簡単にレビューする感じ

GitHubのワークフロー

目的

  • 安全に、素早く
  • 並行開発を簡単に

ブランチ運用

  • master (ブランチを切る元のブランチ)
  • production (本番サーバーにデプロイ)
  • staging (stagingサーバーにデプロイ)
  • feature/xxxx (機能追加用)
  • fix/xxxx (機能修正用)

ワークフロー

  • masterから作業ブランチ(feature or fix/xxxx)を切る
    • 作業内容に合わせて説明的な名前をつける
  • stagingへのプルリクを作成する
    • 差分が全くないと作成できないので、空でcommitしてから
    • タイトルに (WIP) とつける
    • descriptionに目的、作業予定内容(あればチケット番号)などを書く(項目は要検討)
    • 作業レビュー依頼をする
      • レビュー終わらなくても作業入ってOK (作業始めます連絡くらいの位置付け)
    • 作業する
      • 途中でも相談とかしてOK (「このcommitのこの処理なんですけど・・・」)
  • 終わったらコードレビュー依頼をする
    • タイトルの (WIP) 消して
    • 指摘がなければマージ、あればそれを修正して再レビュー
    • 修正中はまたタイトルに (WIP) つけておく
  • stagingでの動作に問題なければ作業ブランチをmasterにマージ
    • masterマージ後は作業ブランチを削除する
  • 本番リリースは、masterをproductionにマージする 

意識するとよいこと

  • stagingにマージ後はすぐに動作確認 → masterにマージ
    • コンフリクトした時にcherry-pickしないといけなくなる → 履歴汚染
  • commitは意味のある最小単位がよい
  • commitはすぐpush
    • 作業ブランチ上であればいくらでも改変してOKなので
    • 作業の状態を共有できる → レビューをもらえる機会が増える
  • プルリクの説明に必要な情報を入れる
    • 特に見てほしい箇所
    • 動作確認できるURL
    • 適宜レビュアーに合わせて前提条件なども

こんなときどうする

  • コンフリクトした
    • masterにrebaseする
  • 本番で緊急に修正が必要
    • masterからhot_fixブランチ切って修正commit後、 masterにプルリク する
    • masterにマージ後、productionにマージ
    • その後stagingにもマージ
  • レビューで指摘された部分、ごもっともなんだけど修正してる時間ない
    • 基本は直したい、けどケースバイケース
    • そもそも、そういった事情を共有しておく
    • 対話