最近 github がよく使われるようになって Pull Requestによるコードレビューが頻繁に行われるようになりました。
フリーのエンジニアは機能の実現だけでなく、コードのみやすさや整合性まで気にしないといけない時代になったとなあと感じます。
もともとコードスタイルにこだわりのある人からすれば当たり前のことばかり書いてるかもしれないです。
自分以外に苦労している人がきっといそうな気がします。その人のヒントになればなと思い書いてみます。
社員とフリーエンジニアの顧客の違い
社員のエンジニアにとっての一番の顧客は担当ディレクタであることが多いです。彼らの顧客が求めるものは機能がいつまでにその機能が実現できるかということです。
フリーランスのエンジニアにとっての顧客は誰でしょう？ 多くの場合は、そのチーム内にいる社員のエンジニアではないでしょうか。
フリーランスのエンジニアは、顧客である社員のエンジニアに満足してもらえるコードと機能を実現する必要があります。
github の普及による求められる能力の変化
github が業務で一般的に使われるまでは期限内に機能を実現しバグがないコードが重要でコードスタイルなどはあまり注目されていませんでした。
pull request が業務に当たり前に行われるようになてからコードの読みやすさや整合性に注目されるようになってきました。
社員に満足してもらう Pull Request とは？
そんなに読みずらいコードを書いてるつもりもなかったので最初の方は、ああ、pull request やるのね、ぐらいで望んでいてたのですが、本気でレビューをする人たちと仕事をする中でいくつか気づきがありました。
コードスタイルが既存のコードと揃っていること
指摘を何回か受けるまでは機能が動けばどうゆう書き方をしてもよいと思ってました。そのため、指摘によってすでに動作しているコードを変更を要求されたこともありました。自分の書き方の方が良いと感じていたり、わざわざ書き直す意味があるのかなどフラストレーションがたまりました。
今となっては当たり前なのですが、この問題は google_javascript_style_guideのあとがきに書かれていたことを読んで納得できました。
一貫性をもたせてください
あなたがコードを書くとき, どのようなスタイルで書くかを決める前に, すこしまわりのコードを見るようにしてください. もしまわりのコードが算術演算子の両端にスペースを入れていれば, あなたもそうすべきです. もしまわりのコードのコメントが, ハッシュマーク # を使って矩形を描いていたとしたら, あなたもまたそうすべきです.
コーディングスタイルのガイドラインを策定することのポイントは, コーディングの共通の語彙を持って, どう書くか ではなく 何を書くか に集中できるようにすることです. 私たちはここでグローバルなスタイルのルールを提供したので, 人々は共通の語彙を得られたことになります しかしローカルなスタイルもまた重要です. もしあなたが追加したコードがあまりにもまわりのコードと違っていた場合, コードを読む人のリズムが乱されてしまいます. それは避けてください.
もし異なるコードスタイルをとり入れたいなら一貫性を持たせるために他の箇所も修正すべきなのだと学びました。
この一貫性をどこまで守るかというのは難しいところですが、できるだけ合わせるのがその現場にとってのよいコードです。
新規で構築する場合
新規で作成する場合は既存のコードがありません。ただ最終的にはそこで働く社員のエンジニアに納品することになります。彼らがメンテナンス可能なコードスタイルで書く必要があります。
新規の場合は、自分のコードが どのコーディングスタイルにしたがって書いてるかの参照元を READ MEに記載することにしています。例えば React であれば、airbnb にしたがって書きます、と宣言します。
最初にコーディングスタイルの合意をとっておけばコードスタイルの指摘があったとしても正解があるので話がもつれることもありません。
lint, prettier の活用
lint, prettier は本当に便利です。コードスタイルの違いの指摘や修正を自動化することができます。pull request をするプロジェクトでは必須だと思います。
特にフリーランスの場合はコードスタイルを揃える必要があり、なくてはならないツールだと思います。
テストコードは自分のためにも書くべき。
テストコードの書く書かないのはプロジェクトによるもので、あまり書かない傾向に昔はありました。
しかしながら pull request によるレビューが行われるようになってからは細かいミスがあるたびにレビューしてもらうのは非効率なので単体レベルの試験は終わらせてから pull request すべきだと思います。
あとテストコードがあれば作成したモジュールがどうゆう使い方を想定して書いたのかもテストコードがサンプルとなり伝えることができるようになります。
最近はテストファーストぐらいの勢いでテストコードを書くようになりました。最初はテストコードに書き慣れていなかったこともあり、結構大変でしたが慣れてくるととても効率的だと気づくようになりました。
コメントやログ出力、コミット時のコメントについて
コメントにも一貫性が求められるので既存のコードを参考にして書くようにしています。一貫性がない、新規の場合も何かしらルールを設定する必要があると思います。必ずしもではないですが、ルールの指定がない場合は、docs を出力できるぐらいの粒度で書くようにしています。
ログは残さない方が良いという意見もあるとは思うのですが、僕は他の人が作業に入りやすいようにそのプログラムがどうゆう動作をしているのが動かして把握できるレベルのデバッグログは残すようにしています
その際も既存のルールがあれば従い、なければ一定のルールで出力します。
例えば初期処理が走った時、イベントが来た時、など出力フォーマットも合わせてフィルタリングしやすいようにして残します。
単純な処理でもライブラリはできるだけ使った方が良い。
ここは意見が分かれるところだとは思うのですが、僕はライブラリを使って実現できることはライブラリを使うようになりました。なぜかというとコードが読みやすくなるからです。また自分で書く量が多ければ多いほどレビュー量も多くなります。
それならすでに github 上で様々な人の指摘を受けて改善されているライブラリを使う方が良いと思います。
もちろん使うライブラリはテストコードが書かれてコードも美しく保たれているものを選択すべきです。
仮にそのライブラリの選択を指摘されたとしても、別のライブラリに差し替えることもできるので手戻りが少なくなります。
注意点
ただ一つ気にしなくてはいけないのは、ライブラリを使うからには利用方法などは作者が書いた正確なドキュメントにしたがって使うべきです。
以前、コードから仕様を把握していたのですが、ドキュメントの指定にあるインターフェイス以外の使い方をするとどこかで変わってしまう可能性があると指摘されました。
それ以来、readme やドキュメントをよく読み、自己流の使い方をしないように気をつけています。
WIP (woking in progress) の Pull Request
Pull Request の完成度にこだわるとなかなか Pull Request までたどり着けない場合があります。そういった場合は、作業中のものを push して WIP の Pull Request をあげて進捗を示すと依頼者は安心します。
WIP のコミットは作業状況を示すために行なっているので、WIP を外す際は最終的に見て欲しい単位にわけて force push して上書くようにしています。
WIP を外してレビューが始まったあとは、force push は厳禁です。レビュー記録が消えてしまってレビューのやり直しが発生してしまうからです。レビュー開始後は、そのあとの修正の流れが細かく見れた方が良いです。
master の反映は、merge か rebase か
一時的 rebase が話題になった時に、rebase を使って最新にしていたのですが、merge を使うようにしました。
理由は下記の通りです。
- master をおいかけたというのが一目でわかるため。rebase だとその変更が残りません。
- pull request 上では、merge ログが見ずらくはない。確か rebase はログが綺麗に残るというので使われていた記憶があるのですが、github を使うと merge ログは見やすく処理され、見ずらくはないです。わざわざrebase を使ってログを組み替える必要がないと最近は感じます。
コミット時のコメントについて
以前コミット時のコメントについて指摘を受けました。
良いコミット時のコメントって何か？というのを考えたのですが、そのコミットはどの目的で行われたものかがわかるものが良いのだと思います。
例 XXX機能追加、XXXのリファクタリング
など。
具体的にいうとコミットログはコードの変化を俯瞰で見たり、何かしら問題があって過去に戻して調査したり、ログの検索したりするとき重要となります。
もちろんすでにあるコードのコミットログが一定のルールで書かれている場合はそれを踏襲すべきです。一定のルールがあった方が探しやすいからです。
コミットログ全体でログからコードの動きを把握できるようになっていれば、それは良いコミットログだと言えると思います。
Pull Request の本文について
変更したことはその内容と目的がわかるように過剰書きで全て書き出す。
これがないとレビュー者がコードを読みながら想像することになります。
レビューする人ができるだけコードをさらっと読み流せるようにリスト化して書き出すの良いと思います。
最後に
色々書きましたが、フリーランスに限らずコードの読みやすさがとても重要になってきたと感じています。
業務でもオープンソースに pull request するぐらいの気持ちでコードを書く時代がきたのかなと思っています。
フリーランスはこういった時代に対応するために、いろんな人のコードを読んでおくのが良いと最近実感します。
既存プロジェクトの場合はそのスタイルを把握するのはもちろんのことですが、グローバルでみたときのコードスタイルも重要なのかと思います。
自分が良いと思っているコードは、なぜそう考えるのかという背景を説明できないと説明ができないので、いろんな根拠を持っておく必要があります。
また自分はこうゆう時どう書くのが良いのだろう、っていう時にメジャーなオープンソースのコードの書き方などをできるだけ参考にすることで様々な学びがあります。コードスタイル以外にも便利で活発に改良が行われているライブラリの発見や良い書き方も勉強できます。
外部組織のコードを触る場合は、コミュニティが存在するオープンソースに Pull Request するぐらいの気持ちを常に持っておくのが良いと思います。そういった気持ちでコードを書くようになって目線がだいぶ上がったような気がします。
こうゆう気づきを与えてくれた今まで一緒に仕事をしたエンジニアの方々にはとても感謝しています。