LoginSignup
9
1

More than 3 years have passed since last update.

ライブラリの定期アップデートのPRをどこまでレビューするか

Posted at

前提

あくまで私がどこまで見てるか、です。会社・チーム・文化によって違うと思うので参考程度に見てください。
今まで Ruby の開発がメインだったのでその経験をもとにしています。

思い出しながら書いてるので、えいっと書き出したものなので、書き漏れがあるかもしれないのですが、それはご了承ください。
また、表現が適切じゃない箇所もあるかもしれないのですが、そこもいい感じに読み取っていただけると助かります ><

しない話

  • 定期アップデートをなぜやるのか
  • 定期アップデートをどうやるのか

レビュー時にみるポイント

  • どれくらいバージョンが上がっているのか
    • メジャーなのかマイナーなのかパッチなのか
    • 現在のバージョンはいくつか
  • production 用か devleopment 用か test 用か
  • 言語系か小さなツールか
  • 作者・チームは信頼できるか・きちんとメンテされているか
  • changelogはきちんと書かれてるか
    • どういう変更か・breaking changesがあるか
    • feature/bugfix/breaking changes/docs etc
  • リリースされてからどれくらい経っているか
  • PR/commit/issue/diffをみる
  • テストでカバーできてる範囲の変更か
  • 仮にバグっていたときどこに影響が出るのか

どれくらいバージョンが上がっているのか

  • APIの変更に互換性のない場合はメジャーバージョンを、
  • 後方互換性があり機能性を追加した場合はマイナーバージョンを、
  • 後方互換性を伴うバグ修正をした場合はパッチバージョンを上げます。

多くのアプリはこの semantic versioning を採用しているはずなので、
メジャー・マイナー・パッチのどれがどれくらい上がったか、をみるば影響範囲が概ね想定できます。

※ ただ、アプリによっては、 semantic versioning っぽいバージョンの付け方をしていても、 semantic versioning に則ってないこともあるので注意です。

また、0.y.zのようなメジャーバージョンが 0 系のアプリに関しては、破壊的変更が入りやすいので注意深く見ます。

production 用か devleopment 用か test 用か

https://bundler.io/v2.0/man/gemfile.5.html#GROUPS
Ruby の Gem (ライブラリ) 管理ツールである bundler では、
以下のように、gem をどの環境で使うのかを設定できます。

gem 'rails'

group :development, :test do
  gem 'byebug'
end

group :production do
  gem 'pg'
end

production で使う gem であれば、もちろん慎重にいきますが、
developmenttest でつかう gem であれば、多少雑にレビューをしてバグったとしてもバグってから直せば良い、という判断をすることもあります。

ただ、developmenttest でも、『通っちゃいけないテストが通るようになって本番にバグが紛れこんでしまう』ということも否定はできないので、
development test でも十分にレビューが必要なケースはもちろんあります。

言語系か小さなツールか

たとえば rails のような影響範囲・依存度が高い gem はとても慎重にレビュー・テストを実行した上でのマージをするケースが多いですが、
おそらくテストのダミーでターでしか使われないような faker であれば、テストが通ってれば十分だろう、という判断してさくっとマージしてしまうことが多いです。

作者・チームは信頼できるか・きちんとメンテされているか

railsrack などのような、大きなコミュニティによってメンテされている gem であれば、changelog をざっと眺めて BugFix しかないのであればさくっとマージしてしまいます。
逆に、あまりきちんとメンテされてなさそうなそうでない gem の場合、きちんと changelog だけでなく commit/diff を丁寧に確認します。

※ もちろん、OSSとして公開してもらえていて、それを利用しているのでありがたやという気持ちでいっぱいです。

基本的には、外部のものは自分たちの作業の範囲外なので信用しない・しすぎない、という体でいるのが正しいと思っています。
なので、基本丁寧にレビューして、信用できる gem であれば、少し手を抜いても良いかもという立ち位置です。

changelogはきちんと書かれてるか

https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md#530-alpha39-november-5-2019
のように、 Breaking Changes, Features, BugFix のようにグループ分けされて書かれてるととても安心感が高くなります。
きちんと書かれてる場合、たとえば Breaking Changesとあればそこは重点的に見ないといけないでしょうし、BugFix だけであればざっとchangelogを眺めて気になるのがなければ changelog だけ眺めてマージしています。

※ 自分のアプリがバグを踏んでいないか・バグ前提になってないなどはもちろんきちんと確認する必要はあります。

でも、changelog が書かれてない・あまりきちんとメンテされてないケースもたまにあるので、そのような場合は、changelog だけを参考にはせずに commit 履歴や diff を確認したほうが良いでしょう。

また、Featuresなどであれば、自身のチームのアプリに導入できるものがないかを見て、導入できそうなものがあれば導入をします。
rubocopのようなlint系のツールの場合、新しいルールをどうするかをチームで検討するなどもします。

PR/commit/issue/diffをみる

changelog だけでなく、基本的には PR/commit/issue/diff もみます。
全部見てると時間が足りなくなってしまうので絞ったほうが良いと思いますが、
Breaking changes, BugFix, Feature などは、どういう経緯・意図なのか、影響範囲はどれくらいか、などを把握するために見ます。
また、他人のコード・PR・issueをみることで学べることはたくさんあるので、そういう観点でも時間があったり興味があるのは見たりします。

(『見てもわからない >< 』、っていう方ほど理解するために頑張ってみるのをおすすめします。)

人・チーム・文化によっては、diff全部見るべき、という意見の方もいると思うのですが、
その意見は十分に理解できますが、現実的には時間が足りないので、うまい落とし所を見つけていく必要があるなと思っています。

リリースされてからどれくらい経っているか

定期的にupdateしている場合、新しいバージョンがリリースされてから1日もたってない gem のアップデートということもたくさんあります。
OSSとの向き合い方として、そういうのを積極的に取り入れてバグを発見したらバグ報告したり修正のPRをあげたりコントリビュートしたほうが好ましいと思っています。
ですが、現実的にはそうはいかないケースもあります。
内容次第では、1週間ぐらいアップデートをスキップして1週間後にバグ報告などが増えてなければ gem をアップデートするという選択もたまにします。

テストでカバーできてる範囲の変更か

普段のレビューの観点と同様です。

例えば、 faker gem であればテストが通っていれば十分だと思います。
でも、たとえば pumawhenever のような、単体テストではテストしづらい gem もあると思います。
それらの場合、実際動かしてみたりして挙動を確かめます。

仮にバグっていたときどこに影響が出るのか

普段のレビューの観点と同様です。

これをイメージするには、gem のことを知っていて、自身のアプリの全体像を把握してる必要があります。
わからない場合は、チームメンバーに相談します。

バグが起きたときにどう検知・対応できるか、どのタイミングで起こり得そうか、みたいなことをイメージし、
「問題が起きたら直す!」って責任が持てそうであればエイヤッとマージしてしまうこともあります。
マージ後に挙動確認して、ダメだったら即revertします。

とくに、とりあえずマージしてデプロイが無事完了してアプリが起動すればOKだろう、っていうものの場合はとりあえずマージという判断をすることそこそこあります。
(動かしてみないとわからないなら動かしてみればいいじゃん、です。)

CI通らない・アプリが動かなくなったときにどうするか

  • Changelog, それに関連する PR/issue/commit/diff を読む
  • 同様の問題報告などがないか、 issue/PR を探す
  • issue/PR を上げる
  • アップデートを見送るのか・パッチをあてるのか

Changelog, それに関連する PR/issue/commit/diff を読む

まずはきちんとどういうアップデートが行われたのか確認します。
Breaking changesBugfixにある内容を踏んでるケースが多いので、注意深く確認します。

同様の問題報告などがないか、 issue/PR を探す

自身のアプリが、その gem にモンキーパッチをあてたり・特殊な使い方をしてない限り、
同様の問題は他の人の環境でも起きてる可能性が高いです。
なので、すでに別の方が issue/PR を上げてくれている可能性があるので、それを探します。

issue/PR を上げる

だれも issue/PR を上げてないのであれば、issue/PRを上げます。

アップデートを見送るのか・パッチをあてるのか

たとえば、バグを発見してすでに報告済みで数日待てば修正されたバージョンが上る見込みがあるのであれば、
それを待つという選択を取ることは多いです。

しかし、アクティブにメンテされてる gem ばかりではないので、すぐに直る見込みがないケースも十分にあります。
その場合は、アプリにモンキーパッチをあてたり、gem をフォークして修正してそのバージョンを使うなどもあります。

どちらにするかは、状況によるのでチームメンバーと相談しながら決めていきます。

最後に

慣れるまでは時間がかかるしハードルが高いと思います。
ですが、根気よく続けていけば、技術力も上がるはずですし、
良い意味で手を抜いてレビューできる部分がわかってきてそこまで負荷なくレビュー・マージできるようになると思います。

あくまで、私自身の経験としてライブラリアップデートのPRをレビューしてる経験から書いてみました。
会社・チーム・文化によって重視する点が変わると思うので、あくまで一つのやり方として参考にしてもらえればと思います。

9
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
9
1