他のチームのコードにプルリクエストを出すときに考えていること

Nov 15, 2022

社内の他のチームがコードオーナーになっている機能やアプリにプルリクエストを投げる時、自分はこんな感じで仕事を進めている (前提として数百人以上のエンジニアがいるような組織で、自分が詳しくない別チームの領域に変更を加えたい場面を想像して欲しい)

  • 変更を加えたい箇所のコードを読み込む
  • 似たような目的のコードがどんな風に書かれているか確認する
    • どんなクラスで構成されているか
    • どんな命名パターンでやっているか
    • どんな風にテストコードを書いているか
  • 上記を踏まえて自分が目的としている変更を加える

この時の自分のメンタルモデルとしては、以下の感じ

  • いかにコードオーナーとなるチームとの摩擦を避けるか
    • よっぽどの明確な利点がない限り、できるだけ新しいパターンを持ち込まない
    • できるだけ既存のコードの書き方に寄せる
  • 自分ならこう書くかな〜?とかちょっと冗長だなーと思っても、基本的には従う
    • コードオーナーに比べてそのコードの歴史に詳しくないので、自分の判断が間違っている可能性が高いと思う
    • 自分がマージして欲しい変更を受け入れてもらうことの優先度が高いので、そこと直接繋がらない議論は避けたい(もちろんパフォーマンス懸念があるとか、セキュリティに問題があるとかなら別)

とはいえ、これをやっているとそうやって既存のパターンをなぞった箇所に対して、自チームのエンジニアから以下のような指摘をもらったりする

  • こうやって書いた方が良くない?
  • この書き方冗長じゃない?

これに対する返信が「既存のパターンにできるだけ寄せて書いた」になリがち。。。この回答は微妙だと思うし、自分もエンジニアとして本当に正しい姿勢なのかとも思ったりする。エンジニアとしてはもっと自分が理想とするコードを追い求めて、それが自分たちのチームがオーナーでない部分に対してもぶつけるべきなのかもしれない。

ただ自分のチームがオーナーとなっている機能やアプリにプルリクエストをもらう立場を考えると、既存のコードと全く異なる書き方や・似たような機能があるのにそれに沿わない書き方をしていたら、さっとマージするのは難しいだろうなとも思う。大きな会社で他のチームのメンテする機能にプルリクエストを投げるのは、他人のOSSにプルリクエストを投げるのに自分の感覚としては近い気がする。

仕事を早く終わらせるという意味だと自分のやり方は間違ってないと思うのだけど、コードを改善するという意味では既存のパターンも疑ってもっと拘って書いた方がいいのだろうか。nitpickではあるけど、指摘としてはまあ妥当、だけどこのプルリクではやりたくないなーーーというコメントをチームのメンバーから自分のプルリクエストにたくさんもらって、そんなことを午後にグルグル考えていた。