jsk-ros-pkg / jsk_robot

jsk-ros-pkg/jsk_robot
https://github.com/jsk-ros-pkg/jsk_robot
73 stars 97 forks source link

[Fetch]Fetchに関するPRの出し方について #1484

Open sktometometo opened 2 years ago

sktometometo commented 2 years ago

現在、knorth55/fetch15 を開発ブランチとしてjsk_fetch_robot以下の開発が行われていますが、jsk-rospkg/masterへpull request を出す際に以下の問題があります。

これを解決したいです。

master-with-pull-requests ブランチ

現在、この master_with_pull_requests ブランチには以下のPRがマージされています。

以下のPRは jsk-ros-pkg/master と conflict があるためマージしていません。

以下のPRは互いに干渉して conflicts を起こすためマージしていません。

この上で Pull Request を出す際のワークフローについて議論したいです。 個人的には手戻りを起こす&発生する可能性があるpull requestが現在のように大量に出ている状態で jsk-ros-pkg/master に pull requestを出す気はあまりないです。

sktometometo commented 2 years ago

@knorth55 @708yamaguchi @mqcmd196 @tkmtnt7000 @k-okada

k-okada commented 2 years ago

マージされていないのは @k-okada 不作為というのもありますが 1) そのPRがどれぐらい必要なのかわかりずらい。 2) 各ロボットで同じような開発がされているのでDiscussonを希望したい。

1)は

k-okada commented 2 years ago

Milestone を活用するのはどうでしょう。 -> https://github.com/jsk-ros-pkg/jsk_robot/milestone/4

knorth55 commented 2 years ago

@sktometometo master-with-pull-requestはconflict確認用ってことかな? conflictはどうしても起こるので、その場でちょくちょく直していくしかないですね。。。 knorth55/jsk_robot@fetch15は人力で北川が直しています。

はじめに:鶏と卵

個人的にはPRは自分が思う単位で出してもらえるとありがたいです。 たしかにfetch15origin/masterからかなりのcommit数があって、絶望感からPRを出さない、多くのcommitが入ったPRを出すには大変すぎる、というのは個人的な感想です。 なので、根本的にはorigin/masterとのcommit数のdiffを減らすことなんじゃないかなと思います。 で、それをやるためにはPRを出さないといけない、というお話しで、鶏か卵かという堂々巡りに入りましたね。 さてどうしましょう?

PRの分類

PRを分類してみると大きく下の4つくらいに上げられるのかなと思いました。

致命的なバグは比較的commitも少ない傾向にあり、PRも出すのが簡単で、結構せっつけばすぐにマージしてもらえている印象があります。 現在の機能のリプレイスはcommitが多くなる可能性もありますが、新たなことができない限りはあまりマージされる見込みもないので、自己満足と言えるでしょう。 問題は新たな機能を追加するPRと/etcなどのロボット設定の変更のPRかなと思います。 前者はたとえばmove_baseの機能をいじる、controllerを変えるなど、後者はsupervisorのジョブを変えるなどですかね。

新たな機能を追加するPR

個人的に一つ解決策として考えていたのは#1149 のようにデモ単位でPRを出すことです。

1149 には多くの依存があるのですが、デモが動いていることを示せばマージしてもらえるかな、と思っていたのですが、そういうわけではなかったようです。

(ちなみに #1149 は最新にアップデートされていないのでマージされるとおそらくconflictの原因になります) となると、デモが動いて論文にならないとマージされないのかな?という仮説が立てられます。 この仮説に対して北川はまだ論文を書けていないので検証が行われていないのですが、塚本くんの卒論や大日方くんの卒論がマージされたのかな、とみていると、卒業論文ではダメ?なのかなというのが所感です。 以上から、新たな機能を追加するPRについては、マイルストーンを決めて論文を発表していくという岡田先生の提案があっていると個人的には思います。 (北川みたいに信頼を失うとどんどんマージされなくなるので、論文を発表してマージしてもらわないといけないですね)

/etcなどのロボットの設定を変更のPR

ロボットの設定を変更するPRが一番厄介かもしれません。 これは新たな機能を追加するという面がある反面で、現在の機能をリプレイスしているという面も大きくあるからです。 実際にFetchのMelodic移行に関する/etc以下の変更のPR #1208 のみがgo-to-kitchen #1149 の中でマージされずにいます。 ほかにも #1208 #1323 #1328 #1332 #1337 #1465 などがあげられます。 これも #1328 のケースをみていると、マージがされないことはない、という感じですが、これには以下の2つが懸念されていてマージされないのだと考えています。

1つめの安定的に動くかについては、新機能の追加と同じでマイルストーンを決めていく同様の手法が有効に思えます。 問題は2つめの「すぐに設定が変わるのではないか」です。 これについてはLinuxの諸々が変わっていくこと (i.e. init.d -> service -> initctl -> systemdやnetworking -> netplan) への懸念と、最適なものはこれだという意見の相違という2つが主に挙げられます。 顕著に現れているのが systemd v.s. supervisordに関する問題です。 現にsupervisorに関するPRは、岡田先生はどうせsystemdへ移行するからそっちに移行するのをずっと待たれているように感じますが、Fetchユーザからはsupervisorで動いているからまぁいっか、という感じで平行線を辿っているように思えます。(c.f. #1323 #1465) systemd vs supervisordに関しては、各ロボットに対してroslaunchが1つであればsystemd, supervisordでもどれでもいいのではないか、というのが私の感想で、その議論については #1465 で行われていました。 個人的にはsystemdでもsupervisordでもどちらでも同じ挙動なのでどっちでもよいと思っているのですが、github上での議論が発散してどこにも結論がない、というのが現状だと思います。 これについてはオフラインで議論するのが手っ取り早いと思いますが、じゃあ誰がやるのか?という話になると現状動いているもののリプレイスメントなので、誰かに勧めることもできず、だれかがやるのを皆が待っている状態になっているかと思います。 最初に海に飛び込むペンギンと同じですね。えいやとやった人が損をするという構図です。 なので北川がやってしまってよいと個人的には思っているのですが、それをやるから後輩がうんぬんかんぬんと言われるので、うーん、どうしようかなという気持ちでもあります。

また#1208 の例にあるように、Merge afterと書いてあるのに先にマージされてしまい、放置されてしまう傾向もあるようです。 先にマージされてしまったケースは残り滓しかPRに含まれておらず、現状どうすることもない、マージされるのは望み薄だなぁという感想です。 なにかのときにうまくマージされたら嬉しいな、くらいが個人的な感想です。

さいごに:jsk_robotはリリースというマイルストーンを失った

jsk_robotindigoまではdebをリリースされていました。 しかしbaxterfetchなどリリースをしないパッケージが存在するため、kinetic melodicではリリースをされなくなりました。 このリリースの機会というのが一つのマイルストーンだったはずですが、この機会をjsk_robotは失ってしまいました。 jsk_robotをソースで入れるというのが当たり前になった今、branchをわけてそのbranchをビルドすればいいわけで、そうなるとorigin/masterが活発にマージされないレポジトリがbranch基準で開発されていくのは開発者としては当たり前になってしまって、origin/masterからの乖離が多くなっていくのかなとおもいます。 リリースでもいいですが何かしらの時期的なマイルストーンを設定することは良いと思います。