hitode909の日記

以前はプログラミング日記でしたが、今は子育て日記です

承認ではなくて、よさそう、と思って暮らしている

普通に書いたdiffは、関心がさまざまなところに散らばっていたたり、書きかけだったりで、意味のまとまりがないもので、それを整形して、説明を書いたものがPull Requestであり、コードレビューは、そのまとまりごとに、他人から見て理解可能であるという承認する行為、という理解をしていた。
なので、レビューを通すことは、動くことに賭けて、以後、動かなかったら責任を取る、みたいなイメージはあまり持っていなかった。
レビュワーの責任をどこまでと規定するか考えて、責任が大きい順に並べていくと

  1. レビューを通した以上、以後は私の責任です、という態度
    • 職人魂を感じる
  2. 見たところよさそうに思いました、という態度
    • 通りすがり風情を感じる
  3. まったくの無責任なので、工数最小化のために何が来てもapproveする、という態度
    • やっつけ仕事

かるぱさんのチームでは1になっているのかな(追記)こうなっている、ということではなくて、こういう感覚に近い人がいるのか?というくらい、とのことだった。
だとしたら、DevとOpsがわかれていて、コードを放り投げると運用してくれる、みたいなモデルに近そう。
そのモデルを推し進めていくと、DevとOpsがサイロ化しているチームでは、DevがめちゃくちゃなSQLを発行してもOpsがサーバーを増強して動かしてくれるように、めちゃくちゃなコードを書いてレビュー依頼に出したら、以後、レビュワーがsuggest changeし続けて直してくれる、ということになって、コード書く側はやっつけ仕事でいいことになってしまう。


うちのチームでは、レビューの時点では、私が見ても理解できました、よさそう、Looks Good To Me、みたいな、上のモデルでは2で、それでも壊れてしまったときは、誰の責任、ということはなくて、壊れてたらユーザーさんが困るので、みんなで寄ってたかって直しているように思う。
変なコードを書いてもCIが落ちずにそのままリリースしてしまう、ということは開発フローが悪いという、チームとしての課題でもある。
SLOは定めてはいるので、本当は、SLO的にギリギリなので慎重に(1の態度)、とか、今週はSLOに余裕があることだし、壊れても大した影響のない部分なので雑レビューで出してみよう(3の態度)、とか使い分けられるとよいのだろうけど、まだそこまでは活用できていない。
ただ、リリース前に延々と詰将棋みたいに議論しているよりは、リリースして動くか様子を見ればいいじゃん、実験できるなら安全に本番環境で実験できればいいじゃん、という想いはあって、リリースできるか、よりは、問題があったら引っ込められるか、ということのほうを重視している。
techblog.karupas.org

コードレビューとPull Request、そしてその承認機能の副作用について考える - 時計を壊せ

あくまでLooks Good To Meであって、私にとっては良いと思います、第三者から見ても大丈夫そう、くらいのニュアンスでapproveしてました

2021/11/08 09:55


ちょっと似たような違う話で、コードレビューのさいに何往復で収束するか、そのやりとりの回数を減らせるとよいことでしょう、ということを以前考えていた。権威的になると、許可を得るために何度もお願いする、みたいになって疲れていきそう。お互い歩み寄って、最速で収束させるぞ!みたいな気持ちが大事だと思う。
blog.sushi.money