コードレビューで聞く質問集
コードレビュー中には、どこを見ればいいんだっけ、ってよくなりますので、 まとめておきました 英語でまとめたものを日本語に訳しました。
- この PR の目的は何なの？
- PR 作成者がこの PR に到るまでの調査結果・決定事項・仮説は妥当か？
- もっといい解決方法はないか？コード修正以外の解決方法も検討。
- この PR は根本的な解決？それとも暫定対応？暫定対応の場合は、PR 作成者に根本的な解決の計画を立ててもらう。
- この変更点はステークホルダーにどんな影響を与えるか？ステークホルダーはエンドユーザだけでなく、チームメンバーとかも含まれてる。
- その影響は最小限に抑えられないか？
- 影響を最小限に抑えるには、具体的にどんなステップを取るか？
- この PR の動作は正しいか？（言うまでもない）
- 足りてないエッジケースはないか？
- セキュリティの穴はないか？
- コードをもっと読みやすくできないか？
- コードをもっと簡潔に書けないか？
- 削除できる不要なコードはないか？
- PR 作成者にもしものことがあったら、レビュワーである あなた はこのコードを引き継ぐ責任は取れるか？
- マージ前後にやることはないか？特に CI やチームメンバーの開発環境に影響を与える修正。
- オンラインメンテでデプロイできそうか？
- デプロイ前後にやることはないか？PR 作成者にチェックリストを書いてもらう。
下記項目は基本確認しないようにしている。
- コード規約・フォーマット。Rubocop などのリンターに任せましょう。
- テスト。上記項目の確認がぜーんぶ終わったら、テストまで確認する気がほとんど失せてるから。
ポジティブなコメントも欠かさず
レビューに慣れてない人は最初 レビュー ＝ 間違い探し と思いがち、ネガティブな指摘しか書かない。問題点が見つからなかった PR も「何か書かなきゃ！」というプレッシャー感じて、些末な nits 書いちゃったりする。そういうときは
- どんな状態からどんな状態に変わったか？良い方向に変わった場合は褒める。読みやすくなったことだけでも。レビューしやすく工夫してくれたことでも。
- コード読んで理解したことを文章化する。理解に時間がかかったときは特に文章化する価値がある。認識の確認にもなる。
このレビュー術は、Aiming で培ってきたものです。
エンジニアの CS 対応メモ
この間、CS からのお問い合わせ対応のやり方について新卒に説教することになりました。 せっかくなので書き落としておきました。
手順は
- 何の問題なのか完全に理解すること
- 不明な場合は、お客様に聞くように CS 担当者に依頼すること。
- 必要があればスクショ撮ってもらう。
- 再現すること（できれば）
- リクエストログが追えれば、追うこと
- 本当にその現象が起きた証拠を集めて参考にするために。その中にある情報で何かが分かるかもしれないから。あと人聞き悪いですが、クレーマーが嘘を付くことがたまにある。
- 原因調査すること
- 解決すること
- CS 担当者に連絡すること
- 砕けた文章で OK。お客様向け文章を書くのは CS 担当者の仕事なので。
- 五月雨式の連絡を避けて、出来るだけ連絡内容をまとめてから投げる。
- お客様に何回も連絡すると、お客様がめんどくさくなる。
- CS 担当者の負担を増やさないように。エンジニアを理不尽なクレーマーから守ってるのは CS 担当者だから。
Rails Needs Active Deployment: 日本語サマリー
先週の Ruby Rogues ポッドキャストで取り上げられた熱い記事。
Active Storage、Active Job、Action Cable、Action Mailbox で超楽になってきたが、デプロイという難関が未だに残ってる。Heroku だと簡単に出来ちゃうけど、それ以外はハードルがかなり上がる。
初心者がやり方ググっても、大量のチュートリアルが出て来て、どれも微妙に違ってて結局自分で大量の決断をすることになり、標準化されてなくて保守しづらいインフラが生まれてくる。
Docker は一見簡単だけど、チュートリアルが大量にありすぎて、ほとんど中途半端かつ開発環境に特化した手順にすぎない。
社内にインフラのプロがいなくても、エンジニアが一人で簡単に本番環境構築できるのは Rails 流ではないでしょうか？Convention over configuration で標準化したデプロイできる Active Deployment 的なものニーズは十分ある。コミュニティで実装方法を議論しましょう。
という主張論の記事。
インフラが怖いと思っている僕はものすごく同感。非現実的だという反論はあるとはいえ。（Ruby Rogues でも実際この点でプチ喧嘩にもなった）
Growing Rails Applications in Practice: 日本語サマリー
読破しましたのでサマリー書いておきました。
- コントローラの設計を標準化せよ
- ユーザーの各インタラクションをCRUDリソースで表現せよ
- コントローラの責務は4つのみ
- 認証
- パラメータのパースとホワイトリスト
- モデルのロード
- ビュー指定
- モデルのAPIをvalidation、コールバックで表現
- ActiveModel のモデル積極的に作れ
- クラス切り出してロジック整理せよ
- ファットモデルをダイエットさせてスリムモデル切り出せ
- インタラクションロジックをフォームオブジェクトに切り出せ
- 他オブジェクトモデルから呼ばれる、Active Recordを必要としない単一責務POROのサービスオブジェクトをきり出せ
- モジュールの使い道はファイル構造化のみ
- モデルを積極的にネームスペース・サブフォルダに配置せよ
- CSSはBEMで整理せよ
- 生きたスタイルガイドを用意せよ
- Railsバージョンアップせよ
- Edgeは2〜3パッチレベル成長するまではバージョンアップするな
- モンキーパッチ、gemが多ければ多いほどバージョンアップのコストが高くなる
- 単体テスト、結合テスト書け
- テスト駆動設計せよ
Ruby Weekly #411: 日本語サマリー
職場の Slack 窓で Ruby Weekly メルマガが毎週配信されます。その中から面白そうなものをピックアップして、日本語で簡単なサマリーを書くようにしています。そのサマリーをここでまとまさせていただきます。くだけた日本語で失礼いたします。
http://rubyweekly.com/issues/411rubyweekly.com
Highlights
Ruby コードのロジックをフロー図に変換してくれる VisualizeRuby gem。
.new メソッドをオーバーライドしてみた話。
#initialize前にシングルトンクラスに対してモジュールをinclude。#allocateで子クラスにもincludeされるように。
Discourse デバッグ中の Sam Saffron 先生が STDERR モンキーパッチでエラー発生元を出力させた話。
Rails 5.2.1 リリース！
require 'bundler/inline' と gemfile ブロックだけで、単独ファイルの Ruby スクリプトでも Bundler 使えちゃう。
Hanami v1.3.0.beta1 リリース。v1.3.0 は 10 月リリース予定。
- RSpec がデフォになった
Articles & Tutorials
dry-rb でサービスクラス実装。
Dry::Initializerで注入する依存性定義。Dry::Validationで注入した依存性のルール定義。Dry::MonadsでSuccessFailureオブジェクトを返す。
Ruby 2.5 の Dir.each_child Dir.children に該当する Dir#each_child
Dir#children インスタンスメソッドが Ruby 2.6 で登場。
partial パフォーマンス向上方法 2 つ。
JIT の紹介。
- JIT とは、Ruby 稼働中によく呼ばれるメソッドの特定・コンパイル
- メモリが倍ぐらいかかっちゃう
- コンパイル完了までは重い
- 今までの JIT 実装
- 許容範囲内の保守性・起動速度・メモリを誇る MJIT は、いよいよ Ruby 2.6 で登場。
テストの期待値は、ヘルパーなどの戻り値ではなく、プリミティブにしようという主張。
Code & Tools
RSpec テスト並列実行用 gem。ファイル単位だけではなく spec 単位でも分散できる。
ActiveRecord や Enumerable を年、月、日付、曜日、時間などでグループしてくれる gem。
Xcode::Install: A Ruby Tool to Install and Update Xcode Versions
XCode 最新バージョンをインストールしてくれる CLI 用 gem。
ターミナルのサイズを検知してくれる gem。
capybara-screenshot: Automatically Save Screen Shots When a Capybara Scenario Fails
Capybara テストが落ちたらスクショを保存してくれる gem。
Ruby Weekly #410: 日本語サマリー
職場の Slack 窓で Ruby Weekly メルマガが毎週配信されます。その中から面白そうなものをピックアップして、日本語で簡単なサマリーを書くようにしています。そのサマリーをここでまとまさせていただきます。くだけた日本語で失礼いたします。
http://rubyweekly.com/issues/410rubyweekly.com
Highlights
指定コミットの中に、テストのカバー率が足りないソースがないか検知してくれる undercover gem の機能紹介。
乱数生成いろいろ。
Scala で新規アプリ開発してみた REA 社が最終的に関数型 Ruby にした話。
- 生産性向上を期待していたが、そこまで変わらなかった
- パフォーマンスは確かに Scala 圧勝
- 関数型プログラミングで保守性の改善を期待していたが、、ボイラープレートが多くて可読性暴落
Rails 5.2.1.rc1 リリース。Rails 5.2.1 は今日中にリリース予定。
Articles & Tutorials
Rails 5.2 Credentials 備忘録。
- config/credentials.yml.enc はコミットしても OK
- config/master.key はコミットするな！.gitignore、.dockerignore に入れること！
- CI ログには
RAILS_MASTER_KEY環境変数を出すな！ - キー変更手順
Ruby 2.6 で Exception#full_message に追加されたオプション引数まとめ。
highlightでエスケープ文字を消すorder
HTML メールの CSS をインライン style 属性として当ててくれる gem まとめ。
- roadie gem
- premailer-rails gem
- actionmailer_inline_css gem
- inline_styles_mailer gem
Ruby 2.6 Adds Option to Not Raise Exception for Integer and Float Methods
変換できない文字列を渡されたら ArgumentError を投げる Integer()、Float() メソッドは、Ruby 2.6 以降、exception オプション引数で例外を抑えれるようになる。
Docker で Rails Credentials を使う方法。
- 開発用コンテナ上で
rails credentials:editが実行できるための設定 - マスタキーの渡し方
Code & Tools
指定コミットの中に、テストのカバー率が足りないソースがないか検知してくれる gem。
簡易 i18n 用 gem。
Ruby Weekly #409: 日本語サマリー
職場の Slack 窓で Ruby Weekly メルマガが毎週配信されます。その中から面白そうなものをピックアップして、日本語で簡単なサマリーを書くようにしています。そのサマリーをここでまとまさせていただきます。くだけた日本語で失礼いたします。
http://rubyweekly.com/issues/409rubyweekly.com
Highlights
認可用 Pundit gem は v2.0 リリース！
グローバルメソッドキャッシュいろいろ。
- メソッドの定義元クラス・モジュールをマッピングするキャッシュ。定義元を毎回評価するのにコストがかかるから。
RUBY_GLOBAL_METHOD_CACHE_SIZE環境変数でキャッシュするメソッドの数を指定（デフォ 2048）。- メソッド数増やしても高速化は 2%〜3% しかない。
ダミーデータ生成用 gem。
Articles & Tutorials
fie フレームワークで ActiveJob の進捗をプログレスバーとして表示。
Sinatra アプリで pundit gem を使う手順。
ファイル名、行番号を返してくれる、Ruby 2.6 の Binding#source_location。
コミット対象の .gemrc の中の認証トークンなどを隠す方法。
コアメソッドに可読性向上の拡張をしてくれた pretty_ruby gem。拡張はモンキーパッチではなく、安心安全な refinement で。
.map { |x| x.join('-') }を.map(:join, '-')に.map { |x| x.next.upcase }を.map(:next >> :upcase)に.reduce(['']) { |m, x| m << m.last + x }.drop(1)を.scanに#take#dropのマイナス引数対応
Rails 開発者必読の書籍 11 冊。
- Clean Code （和訳あり）
- 達人プログラマー （和訳あり）
- Ruby のしくみ （和訳あり）
- Eloquent Ruby （英語のみ）
- オブジェクト指向設計実践ガイド （和訳あり）
- リファクタリング:Rubyエディション （和訳あり）
- The Rails Way （英語のみ）
- Rebuilding Rails （英語のみ）
- Crafting Rails 4 Applications （英語のみ）
- Confident Ruby （英語のみ）
Code & Tools
CLI で四角い枠を描画してくれる gem。
FastJsonapi: A Super-Fast JSON:API Serializer for Ruby Objects
v1.3 がリリースされた高速 JSON:API シリアライズ用 gem。
カバー率測定で、修正したファイルの関連 RSpec テストを特定してくれる gem。
OpenStreetMap API クライアント gem。
Terraforming: Export Existing AWS Resources to Terraform Style
既存の AWS リソースを元に Terraform 用 .tf、.tfstate 設定を生成してくれる gem。
