読みやすいPRの書き方まとめ

前提

コードを書く前に

  • [ ] 実現したい要件はそれで合っていますか?
  • [ ] フローチャートなどで処理の流れを可視化できていますか?
  • [ ] 本当にそのソリューションで正しいですか?解決方法は本当にそれだけですか?

変数

  • [ ] それ変数化する意味ありますか?
  • [ ] その変数初期化する意味ありますか?

メソッド

  • [ ] そのメソッド本当に抽象化する必要ありますか?
  • [ ] その処理再利用できませんか?
  • [ ] そのメソッドその責務負う必要ありますか??

制御文

  • [ ] そのif文三演算子でone linerでかけないですか??
  • [ ] その評価・条件本当に必要ですか??
  • [ ] それ以外(else)の処理書き忘れていませんか?
  • [ ] そのif文、switch文にできませんか?もしそうしたら、break忘れてないですか?

コメント

  • [ ] その処理コメントつけなくてもわかってもらえますか?
  • [ ] 逆にそのコメント処理の内容とか関数名とか見れば自明ではないですか?

名前

  • [ ] その名前抽象的じゃないですか?もっと具体的にできないですか?
  • [ ] そこ本当にキャメルケースでその言語はその命名規則で正しいですか?

基本

  • [ ] ちゃんと要件通りにうごいているかためしましたか?
  • [ ] もっと短く書ける書き方はありませんか?
  • [ ] その書き方は最新の安定版ですか?
  • [ ] 無駄な評価、条件はありませんか?
  • [ ] 文字リテラルの結合処理は + ではなく、interpolation使っていますか?
  • [ ] そのメソッド、変数のスコープはそれで本当にただしいですか?
  • [ ] その関数とかクラス、ちゃんと単一責任ですか?
  • [ ] もう少し早くその関数から返すことってできませんか?fast returnできませんか?
  • [ ] 既存のコードに流されていませんか?どうしてそのフォルダ構造、ファイル名になっているか考えましたか?
  • [ ] もう少しそのコード単純にできませんか?

リーダブルコード

http://tenshotanaka.hatenablog.com/entry/2017/07/05/173544

注意すること

Commit (社内の元cookpadの「R言語上級ハンドブック」共著者の人が言ってたこと転載)

  • どの commit の状態でも最低限動く状態にする
  • 試行錯誤の結果、無意味になる commit は削除する or 1 commit にまとめる
  • 変更の必要性がわかるようにする
    • 例えば、model を追加する commit、controller を追加する commit、view を追加する commit みたいに分けると、commit 順で見ても model のメソッドがどこでどう使われるかがわからないし、controller で取得した情報が view でどのように使われるかわからないので良くない
  • 開発途中で気付いた不要なコードの削除は 1 commit にする or 別 PR にする
  • commitを適切な粒度で分ける
    • 新しいtableを導入して管理画面を実装する際はCRUDの実装をそれぞれ別commit にする
    • 機能の廃止、機能の追加、リファクタリングなどは commit か PR を分ける
    • メソッドの移動等、変更量が多い場合は細かく commit を分ける

Comment

  • プルリクエストには目的を明記する
  • プルリクを読む人々が経緯に精通していると思い込まず、なぜその仕事が発生したのか概要を提供するように努める
  • 会社の誰しもがプルリクエストを読めるということを忘れないようにする。今後、今参加している人以外が読んだ時にも、内容や雰囲気がわからなくてはいけない
  • あなたがどんなフィードバックを望んでいるのか明確にする。例えば「コードに目を通して欲しい」「技術的な議論がしたい」「設計について批評してほしい」「文言を確認してほしい」などです
  • 議論に関与させたい誰かがいるなら@hogeと指定(mention)する。そしてなぜmentionしたのかを添える。(例: /cc @jesseplusplus ロジックの明確化のため)
  • チームに対してmentionする場合も同様に理由を添える。(例 /cc @github/security このやり方で問題ないですか?)

フィードバックへの対応

  • とにかくまず感謝を表明する。
  • 明確化するよう求める(「ねえよく分からないんだけど、もう少し説明してくれませんか?」)
  • 質問されたら、なぜそのような実装に辿り着いたのか説明し、しっかりとわかってもらう
  • できることなら、すべてのコメントに返信する
  • 関連するコミットやプルリクエストにリンクを貼る(「1682851でしたやつ、名案だね!」)
  • 議論の余地があったり混乱が見られる場合は、もっといい言葉をつかって彼らと会話できるのではないか自問してみる。対面で話し合っているつもりでやりとりし、オフラインで起きている議論を要約した続報をお互いに投稿してみる。(これから読む人に役にたちます)

Reference from