Upgrade to Pro — share decks privately, control downloads, hide ads and more …

快適なコードレビューを目指して / For a comfortable code review

快適なコードレビューを目指して / For a comfortable code review

Code Review Meetup #4 の資料です

Kazuma Watanabe

September 27, 2018
Tweet

More Decks by Kazuma Watanabe

Other Decks in Programming

Transcript

  1. パターンを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: | 第三引数を省略したとき、型の比較が行われないので注意してください

    in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき
  2. パターンを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: | 第三引数を省略したとき、型の比較が行われないので注意してください

    in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき 識別子
  3. パターンを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: | 第三引数を省略したとき、型の比較が行われないので注意してください

    in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき レビューコメントのきっかけになる コードのパターンを書く 大体PHPの式が書ける ? は任意の式に一致する
  4. パターンを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: | 第三引数を省略したとき、型の比較が行われないので注意してください

    in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき 何を確認して欲しいのか Gitのコミットメッセージのイ メージで書く
  5. パターンを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: | 第三引数を省略したとき、型の比較が行われないので注意してください

    in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき どんなときに無視して良 いのか書く
  6. 結果 $ phinder ./test.php:3:1 in_array("string", [0, 1]) 第三引数を省略したとき、 型の比較が行われないので注意してください in_array関数の第三引数は省略されたとき

    false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php (in_array_without_3rd_param)
  7. 例: もっと大雑把に書く - id: db_long_transaction pattern: DB::Transaction message: | トランザクションが長くなる場合にはキューを使いましょう

    長いトランザクションを同期的に処理するとレスポンス速度の低下 などの問題を招きます。キューを使うことで非同期に処理ができます justification: トランザクションが十分短いとき
  8. 例: もっと大雑把に書く - id: db_long_transaction pattern: DB::Transaction message: | トランザクションが長くなる場合にはキューを使いましょう

    長いトランザクションを同期的に処理するとレスポンス速度の低下 などの問題を招きます。キューを使うことで非同期に処理ができます justification: トランザクションが十分短いとき 問題とはほとんど無関係なパ ターンだが、確認を促す起点 としては十分
  9. 例: Postmortemと一緒に書く - id: do_not_rename_column pattern: (?)->renameColumn(...) message: | カラムのリネーム時にはサービス停止が起きる可能性があります

    過去にカラムをリネームした際にデータベースにアクセスできなくなった 障害が発生しました。同様の問題を起こさないか確認してください。 参考: https://foo.example.com/postmortem/12 justification: トランザクションが十分短いとき
  10. 例: APIの注意すべきポイント - id: subscription_is_not_active_when_trial pattern: (?)->isActive(...) message: | SubscriptionクラスのisActiveメソッドはトライアルを含みません

    isActiveメソッドは歴史的経緯により、トライアルの場合には falseを返します。トライアルを考慮する場合には注意してください justification: トライアルを考慮する必要が無いとき
  11. 例: テストを書く - id: in_array_without_3rd_param pattern: in_array(?, ?) message: |

    第三引数を省略したとき、型の比較が行われないので注意してください in_array関数の第三引数は省略されたとき false です。 このとき、各要素は緩やかな比較によって比較されます。 参考: https://secure.php.net/manual/ja/function.in-array.php justification: 緩やかな比較を期待するとき test: fail: in_array("string", [1, 2]) pass: in_array("string", [1, 2], true)
  12. 例: テストを書く $ phinder No Error $ phinder `in_array(2, $arr,

    false)` does not match the rule sample.in_array_without_3rd_param but should match that rule. エラーが無いとき エラーがあるとき