はじめに
先日株式会社ハックツさんが主催するハッカソン ツマグロカップに参加させていただきました。
この場を借りてこのような機会を設けていただいたことを感謝申し上げます。
そのハッカソンにおいて開発を行っている際にこれを記事にしようと思ったことがあったので、自分への戒めも含め今回記事にしました。
PRを送る前にする5つのこと
- 不要なコメントアウトの削除
- 不要な標準出力の削除
- 変更点の確認
- ローカルビルド
- 不要な宣言の削除
1.不要なコメントアウトの削除
まずは1.不要なコメントアウトの削除について
コメントアウトはプログラムに影響を与えない開発者が分かるように書くメモのことである。
プログラムに影響を与えないという点から、一時的なコードの無効化にも使用される。
コメントアウトを消せ！という話ではなく、あくまで不要なものを削除するべきという話である。
例えばhtmlにおいて一部要素を一時的に削除したのち、新たに要素を実装するということは多々ある。
その際に一時的に削除した要素をもう一度表示させるのか消すのかをはっきりさせなければいけない。
必要がなければ、削除する。必要ならばコメントアウトを解除し、要素として出力させる必要がある。
つまりコメントアウトを解除した場合それがプログラムに直接影響を与えるものである場合、そのままにするのではなく削除、またはコメントアウトを解除する必要がある。
2.不要な標準出力の削除
次に2.不要な標準出力の削除について
プログラムを書いていると、コードがエラーを吐く、予期した通りに動かないということは多くある。ないに越したことはないがプログラムを書いている以上ほぼ確実に起きるものなので仕方がないと言えば仕方がない。
その際になぜ動かないのかデバックするために標準出力を用いてデータの中身などをコンソールに出力することがあるだろう。
その結果、なぜ動かないのかがわかって解決!そのままマージ！！とはならない。
そのままにしてしまうと本来コンソールに出力される必要のないものまで出力することになってしまうからである。
必要ない出力は消しましょう
3.変更点の確認
次に3.変更点の確認である。
3.1 作業内容の厳格化
これが今回割と大きな問題になっていた。
例えば「Headerの作成」というissueに対してHeaderコンポーネントを作るついでにindex.tsxに不具合が見つかったからついでに修正しようなどと言って同一issue同一ブランチでこの作業を行ってはいけない。
なぜなら作業内容は「Headerの作成」であって「index.tsxの修正」ではないからである。
個人で修正する分には構わないがそれをcommitに含めるべきではない
修正したとしてもスタッシュしたのち、issueを切ってブランチを生やして作業を行おう。
3.2 フォーマッター
またこれもよくあるのだが、開発者のフォーマッターの設定の違いによるコード変更である。
開発者の多くがフォーマッターを使うと思うがこれらの設定は人によって違う。
普通はプロジェクトごとにフォーマッターを設定するものだが、されてないものもある。
これによって自動フォーマットが働き編集したつもりが無いファイルでも保存によるフォーマットでコードが書き変わってしまうことがある。
自動でフォーマットされたファイルも変更点がないならばcommitに含めるべきでは無い
4.ローカルビルド
次に4.ローカルビルドについて
ビルドとは
「ビルド」とはソースコード上に問題がないかどうかを解析を行った上で、問題がなければオブジェクトコードに変換し、複数のオブジェクトファイルを1つにまとめて実行可能なファイルを作成する作業を指します
つまりソースコードに問題がないかを見ることを兼ねた処理である
まあビルドとは言ったが要はコードがエラーを起こさないか見るべきであるという話である。
大型のプロジェクトではCICDを組んでテストをしているものだが一応ローカルでも実行して直しておくと手間が減る。
5.不要な宣言の削除
最後に5.不要な宣言の削除である
よくありがちなのが使用していない関数などのimportである
また削除していないスタイルの適用 style={{}}のような意味のないコードである。
このようなものは意味をなしていないコードなので削除しましょう。
結論
意味のないコード、変更は削除しましょう。
著者はプログラミングを始めて日が浅いため、何かしら間違ったことを言ってたら申し訳ありません。
また他にもこのようなことをやっておくべきだっていうことがありましたら後学のためにも是非教えていただきたいです。