できるエンジニアの先輩からコードレビューを受けがちなポイントを６つにまとめてみた。
1. DRYか
Don't Repeat Yourself. 言わずもがなのrailsの基本指針。自分でコードを書いていて、繰り返しがないかという観点は常に持つようにしましょう。
こちらは、helperを使ってviewをdryに書く方法。
繰り返すのはいけてない！という考えを持って、重複したコードが出てきたときに、どうまとめられるか？という方向で頭働かせましょう。
2. n+1 Queryが発生していないか
n+1 queryとは、レコード数の回数だけQuery発行してるってこと。これが起きてしまうのは、アプリケーション側で、SomeRecord.allでとったものに対して、その子たちを全てアクセスするみたいなことをしてると発生する。下みたいにね。
children = [ ]
SomeRecord.all.each do |record|
children << record.child
end
以下のように、初めからchildをincludeしてquery投げれば、一回のqueryで済む。
（さらに、includesメソッドがrelationを返すのでallも不要になる）
children = [ ]
SomeRecord.includes(:child).all.each do |record|
children << record.child
end
このへんも見る人が見ると、質の低いコードとみなされてしまうので、意識しましょう。
追記
jnchitoさん( http://qiita.com/jnchito )、ご指摘ありがとうございます！上のコードは、本当は以下のようにリファクタできますね。eachで回さずにmapで書くという。こちらの書き方でも読めるよという方はこちらも参考に。
n+1のコード
children = SomeRecord.all.map(&:child)
includeによる改善されたコード。
children = SomeRecord.includes(:child).map(&:child)
3. ロジックをmodelに書いてるか
controllerを書く際に、modelにかけるロジックをcontrollerに書いてcontrollerがくそ長くなるなんてことはないでしょうか。オブジェクト指向的に、メソッドはなるべくオブジェクトに持たせてしまいましょうということで、modelに書きましょう。
最初からmodelにロジックをかけるようになると、後でリファクタでmodelに書き直す時間も省けるし読みやすくなるし、効率的です。
4. 柔軟性があるか
柔軟性の有無は大事です。柔軟性のないってどういう状態かというと、例えば、一つの変更が生じたときに、その変更に応じて書き直さなきゃいけないコードが多数存在するみたいな状態。柔軟じゃない==変えにくいってこと。
変更はつきものなので、常に、柔軟性の高いコードを書きたいわけですが、そのために必要なのは、ベタ打ちを減らす、だったり、上でも紹介したようにDRYに、同じコードを繰り返さないこと。(helperで柔軟なviewをつくる: http://qiita.com/shunsuke227ono/items/21f5968ca7ca0391b583)
この意識があるかないかで、変更が生じたときにどれだけ素早く正確に対応できるかがだいぶ変わります。
5. sql更新処理はtransactionを必要としないか
sql更新処理で、一括に更新されないと困る！なんてときは、transactionを追加してやりましょう。実用的なアプリには必須です。(Transactionのポイントを三つ: http://qiita.com/shunsuke227ono/items/c9c38c24beee33db0a63)
意識せずにコードを書いていると忘れてしまう&別にデバッグでも気づかないので、最初から意識項目にいれて、書き忘れるなんてことがないようにしましょう。結構クリティカル。
6. コード変更による影響範囲はないか
仕様を変更した/partialを書き換えた/modelのインスタンスメソッド書き換えた...etc。基本的に変更がある場合、他の部位に影響があるか？という観点で考える癖をつけましょう。
変更があるか調べるのは割と簡単で、基本的には、変更したメソッド名でgrepかけてやったり、フォルダない検索して一個ずつ潰すなんていう確実な方法もとれます。