sakura-editor / management-forum

管理・運用向けフォーラム(Issues をフォーラム代わりに使う)
2 stars 0 forks source link

PR のマージに必要な approve の数 #60

Closed m-tmatma closed 5 years ago

m-tmatma commented 5 years ago

PR のマージに必要な approve の数は現在 1 ですが、2 に変更しませんか? その PR まだマージしないで欲しかったというのを防ぐためです。

マージに必要な approve 数を増やすことでマージまでに時間がかかることになりますが、 実際にやってみて、問題があれば簡単に戻せます。

protect-branch

beru commented 5 years ago

PRによってはレビューしてくれる人が複数人現れないのが辛いとこですね。

m-tmatma commented 5 years ago

レビュアーが二人だと見逃しを防ぐという効果もあります。 ただ一方で些細な修正の場合は大袈裟であるという面はあります。

beru commented 5 years ago

運用で対処するのはどうでしょうか?

些細な修正の場合は(どこまでが些細なのか判断するのも恣意的ですが)一人のレビュアのApproveで良いと思います。

些細でない場合は一人のレビュアがApproveした後に別のレビュアに要請するのはどうでしょうか?要請をしてもしばらく(数日?一週間?)反応が何も無かったらマージするというのは。

何でこんな事を言っているかというとアクティブな人の数が現在一桁台なので、常に 2 にすると今より過疎化した時に問題になりそうな気がします。

m-tmatma commented 5 years ago

最初に approve を出す人が些細な修正かどうかを 決めるというのもありかもしれません。

PR を出す人が決めるのは、客観性がないかもしれないからです。

その上で必要な数の appove が揃った時点で、PR を出した人が、 マージするかあるいはまだ微調整を続けるのか考えたらいいと思います。

berryzplus commented 5 years ago

PRによってはレビューしてくれる人が複数人現れないのが辛いとこですね。

これかなり致命的だと思います。

レビュアーが二人だと見逃しを防ぐという効果もあります。

そもそも色んなとこに目をつぶってapproveする現状があるので、 2人以上だからといって見逃しの予防につながるかというと微妙です。 細かいところはスルーしてでも通したほうがメリットのあるPRはこれまでたくさんあったと思っていて、実際それらを通してよかったと考えています。

PR を出す人が決めるのは、客観性がないかもしれないからです。

マージするかどうか決める基準は主観でいいと思います。

753 についていうと、#403 をapproveできなかった原因を元に戻したと理解してます。

「コマンドラインの仕様変更はついでにしないで」というお願いでした。 PRが分かれた時点で、コマンドラインの仕様変更が主目的に変わっています。 マージされない状態でこのPRを見たとしたら、いつものように細かい指摘をいくつか書いたうえでapproveしたと思います。なのでそのままマージしても問題はありません。

形式的に「 #403 の変更に対応するため、コマンドラインの仕様を変更する」が #753 の実質になっていると思います。マージ前時点でこんなコメントしたとしたらPRのタイトル変わったりしますよね?

ツッコミが入るのを想定できなかったとは考えにくいです。 全体方針としての approve 数の設定は現状のままでよいと思います。

KageShiron commented 5 years ago

レビュアー2人は現状では難しい条件かもしれません。(私もなかなか確認の時間が取れなくてレビューできてなくて申し訳ないのですが)

多少のリバートやマージ後のツッコミはしょうがないと割り切って、開発速度を優先した方が益が大きいと思います。

m-tmatma commented 5 years ago

では現状維持ということで。