フィヨルドブートキャンプというプログラミングスクールで初めてのPRに対するレビューをたくさんやっています。これらの指摘は世界中で行われていると思うので無駄が減るように書きました。

Files changedを確認したか

Image from Gyazo

真っ先に注意したい点。

ローカルの自分のエディタ上で確認していたとしても、PRを作ったら必ずFiles changedを必ず確認する癖をつけよう。GitHubのソースコードビューワーの機能で色々と気づける点もある。

明らかにFiles changedを一回も見てないなというPRはわかるので職場の先輩に怒られる。

lintを通しているか

rubyだったらrubocop、jsだったらeslint

どういうルールが良いかという高度は話題は置いておいて、まずはlintを通しているか否か。

ファイルの末尾に改行が無いか

これ

Image from Gyazo

どのエディターでも設定で自動的に入るようにできるので設定すること。

同じコミットログが続いていないか

適切な文章などの高度な話題以前に明らかに適当になっている場合がある。コミットログというのはちゃんと見られるものだということを理解しよう。

コミットログが英語/日本語になってないか

プロジェクトのルールに合わせましょう。フィヨルドブートキャンプでは統一されていればどちらでもいいです。

コミットログが雑になってないか

modifyだけ。追加だけは後から見た人が困るので駄目。困る人には1ヶ月後の自分も含まれる。

コミットしたユーザーアイコンがOctcatになっていないか

gitクライアントに設定したemailアドレスがGitHubのユーザーのメールアドレスと違う場合にこうなる。

他のチームメンバーからすると「謎のアイコンの人がコミットしてる!」という見た目になる。

これをやると他のメンバーに「ああ、普段コード書いてない(GitHubを使ってない)人なんだな・・・」という生暖かい視線を浴びる羽目になる。

無意味なファイルがないか

generatorが作成した無意味なファイルは削除しよう。実装のないmoduleとか、中身のないtest、fixturesとか。

「これはgeneratorが勝手に作成したんで」というと先輩にすごく怒られる。一つ一つどんな意味を持っているのか調べよう。

gemのgroupが適切か

testでしか使わないgemがgroup無しで全groupで読み込まれている。必要なところだけにしよう。

不要なrespond_toがないか

jsonでアクセスする必要が無いのにscaffoldのコードをそのままで残っている。htmlだけが必要ならrespond_toformatは削除しよう。削除するときにrespond_toformatが何をやっているのか調べた上で消すのも忘れずに。

何でもリファクタリングと言ってないか

リファクタリングはプログラムの振る舞いを変えずに内部構造を変えることなので、ちょっとした修正という意味で使っていると先輩に怒られるというあるある。

秘密の情報がコミットされていないか

oauthのsecretとか。コード中では環境変数にしておこう。

これをやると、gitで過去を修正したとしてもGitHubのキャッシュに残ってしまい、完全に消すにはGitHubのサポートに連絡するとか面倒なことになる。

チームメンバーが「あ〜あ」という顔になる。

.gitignoreは適切か

.DS_Storeはmacだけのファイル。.ideaはJetBrainsのエディター独自のファイル。そういった特定の環境の人向けのファイルは.gitignoreに含めるべきじゃない。

プロジェクト毎の設定じゃなくて自分のマシン固有の設定に書くべき。

大量の依存gemがコミットされていないか

gemのインストール場所をvendro/bundleとかにしてあって、その中にある大量のgemのファイルがコミットされてしまっていないか確認すること。

ネイティブバイナリが含まれたgemもあるので環境によって違うので各自がインストールするものです。

Comments


(Option)