アプリケーションの保守をやっていると、色んなPull Requestを見る機会がある。レビューする/しないは関係ない。自分の変更を入れる前に過去の実装経緯を確認し、デグレリスクがないか・暗黙的なコンテキストがないか確認したいのだ。もちろんレビュアーをやることもある。解読に時間がかかるPull Requestの傾向が見えてきたのでメモする。
変更点が多い
散々言われているので割愛。コードフォーマッタを途中で導入する場合、コードフォーマット用のPull Requestを作成してほしい。
説明書きやコメントが何もない
保守する側からすると最悪。Issueも真っ白だとヘイトが増してしまう。「今だけ乗り切れば良い」とか、「自分が分かってれば良い」という考えなのかもしれない。ただ、いつかは担当は変わる。そしてコードはメンテされる時間のほうが長い。
せめて何故この変更が必要なのかに関する説明書きは欲しい。これがないと後から仕様変更が発生したとき、コード変更による影響調査に時間がかかる。「よく分からないから残しておくか」とかする人もいる。勘弁してほしい。
テストを見れば良い？こういうことをする人間はテストを書かない。保守性を意識している人間は説明書きもテストもきちんとしている。
説明に書いてあることと実際のコードが矛盾している
「文章と実際の変更内容が矛盾している」パターン。具体的にはAの設計だったがXXの理由でBの設計にしたと説明やPull Requestのコメントに書いてあるにも関わらず、実際のコードにAの設計で書かれたコードが残っているようなものを指す。
特にレビュアーの場合、無視するわけにもいかないので困る。コメントやコードコミットの時間から推測して「消し忘れかしら…」と想像はできる。しかし、それを読み手側に想像させるのは良くない。
変更理由の記載がない/浅い
警告が出ていたため修正しましたとコメントにあったとする。このコメントから、色々疑問が湧いてくる。
- どんな警告？
- どういう修正が必要？
- どうしてこの修正にしたのか？
- 影響範囲は？
コードを読めば分かるのかもしれない。しかし人の思考はコードでは表現できない。何を考えたのか文章でもきちんと書いてほしい。
喋るときに指示語が多い人のPull Requestは解読が大変な傾向がある。国語力なのかもしれない。
