hotwire-love / touhyosan

Hotwire.love で使う投票ツール
MIT License
14 stars 19 forks source link

rails db:migrate etcがwarningでおちるのを回避する (pg1.5.1以降) #53

Closed asip closed 1 year ago

asip commented 1 year ago

pg1.5.1以降を利用していると、rails db:migrate etcが下記のwarningでおちる ので、このプルリクエストで下記のwarningを無視するようにします。

PG::Coder.new(hash) is deprecated. Please use keyword arguments instead!

(このプルリクエストはGitLabのプルリクエストを参考にして(パクって)います。 https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118611 https://gitlab.com/gitlab-org/gitlab/-/commit/6ed866c13a8fbbeef1fb848d979c5c19ba8bf40c )

参考: https://github.com/jeremyevans/ruby-warning https://techracho.bpsinc.jp/hachi8833/2020_07_14/94869#3-1

asip commented 1 year ago

@JunichiIto

14回の議事録のチェックアウトに、自分のチャックアウト時のコメントが上手く 伝わっておらず、自分がコメントした内容と異なる内容となっていました。

14回の議事録(wiki)の訂正のしかた or 訂正してもらうにはどこにコメントすれば よいのか、が、わからなかった、ので、こちらにて依頼させていただきます。

https://github.com/hotwire-love/docs/wiki/Hotwire.love-meetup-vol.14

あしだ
投票さんの仕様がわからないところがあった
pgのwarningが消えなかった
あとで確認したい

あしだ
アンケートの項目を(テキストエリアに)改行で複数指定する(投票さんの)仕様を把握できておらず、その把握に少し時間を取られた
pg(gem)の最新版(1.5.1以降)で発生する(おちる)warningの回避に少し時間を取られた(回避自体は数分で対応)
上記2点のため、モブプロの内容に追えていない部分があったので、あとでコードを確認します

に訂正願います。(上記の文章はちょっと堅いので、フランク?な感じにして 訂正してもらえると助かります。)

また、上記のアンケート項目のように、現状、コードを追わないと把握できない (直感的ではない)投票さんの仕様について、wiki等で明文化してもらえると、 自分のように途中から参加する人間は仕様を把握しやすい、と思います。

アンケートの項目については、画面に「改行で複数指定可能」のような文言を 追加したほうがよいかと思います。

よろしくお願いします。

このコメントは14回の議事録を訂正いただいたら削除する予定です、ので、 このコメントへの返信はなしでお願いします。

JunichiIto commented 1 year ago

@asip とりいそぎwikiは更新しておきました! (文章が堅いかも、とのことですが、どの内容も速記していったので、ページ全体で見れば特に違和感はないかなと思いました。)

PRについて

pg1.5.1以降を利用していると

こちら、現時点ではtouhyousanはpg 1.4.3を使っているので、普通にセットアップする限りpg 1.5.1が使われることはないのかな?と思ったのですがいかがでしょう? また、この問題は以下のPRで解消されているため、これがリリースされれば、それで大丈夫かな、という気がしています。

https://github.com/rails/rails/pull/48048

もし何か他に懸念があれば教えてください〜。

その他

アンケートの項目については、画面に「改行で複数指定可能」のような文言を 追加したほうがよいかと思います。

フィードバックありがとうございます!たしかにちょっとわかりにくいUIですよね。どうもすいません🙏 こちらはそもそも現状の使い勝手が悪いため、UIをがらっと変える必要があるかなと思って以下のissueを立てています。 なので、こちらでまとめて対応できればと思います。

https://github.com/hotwire-love/touhyosan/issues/32

投票さんの仕様について、wiki等で明文化してもらえると、 自分のように途中から参加する人間は仕様を把握しやすい、と思います。

こちらもおっしゃるとおりですね。運営の方で何か対策を検討してみますね。

asip commented 1 year ago

@JunichiIto

とりいそぎwikiは更新しておきました!

ありがとうございます。🙏

こちら、現時点ではtouhyousanはpg 1.4.3を使っているので、普通にセットアップする限りpg 1.5.1が使われることはないのかな?と思ったのですがいかがでしょう?

確かに bundle install のみで bundle update しなければ大丈夫ですね。 自分は bundle update してしまう質ですが。。。

また、この問題は以下のPRで解消されているため、これがリリースされれば、 それで大丈夫かな、という気がしています。

5/25 開催予定の次回の15回(次回)までに、7.0.4.4 or 7.0.5が出る可能性は 高い気もするので、pgの上記の問題は放置するなら放置するで構わないかと。 (7.0.4.2が2023/01/25、7.0.4.3が2023/03/13、のリリース)

プルリクエストが酔っていることが Gemfile にwarning gem を追加して config/initializers/warning_silences.rbを追加して、warning_silences.rbで pg1.5.1以降で出るwarningを 無視する記述をしているだけ、なので、 変更した箇所にTODOコメントを追加して、7.0.4.4 or 7.0.5 がリリース されれば、変更箇所を削除するようにすればよい、のではないか、とも、 思います。

JunichiIto commented 1 year ago

自分は bundle update してしまう質ですが。。。

うーん、それはどうなんでしょう。。 bundle updateはある意味コードの変更なので、機能の開発をするならbundle installだけで開発に着手すべきですし(=機能追加以外の不要な変更を加えない)、gemを最新にしたいならbundle updateだけを実行してその変更だけをPRとして送ってもらうのがセオリーな気がします。

また、このPRについても、bundle updateを実行してから警告をignoreする、というのならわかりますが、警告をignoreするだけのPRは意味がない(なぜならこの状態ではpg 1.4.3で動くので警告が出ないから)と思いました。

asip commented 1 year ago

自分は bundle update してしまう質ですが。。。

うーん、それはどうなんでしょう。。

touhyosan のような実験?用アプリや自作アプリでは、ですよ。

仕事で参加するアプリ開発では、bundle install のみで 必要がなければ、bundle update は行いません。

また、このPRについても、bundle updateを実行してから 警告をignoreする、というのならわかりますが、警告を ignoreするだけのPRは意味がない(なぜならこの状態では pg 1.4.3で動くので警告が出ないから)と思いました。

このPRは自分のようにbundle update してしまう方のための 予防線のPRですね。

bundle update してしまうとwarningに遭遇するので。 (自分は、bundle updateして問題に遭遇しても、自力で解決する ので、問題ありませんが。(更新されたGemfile.lockは必要が なければ、コミットしません、仕事で参加するプロジェクトでは。))

Hotwire.love には、Rails初心者でRailsでの開発の作法に詳しく ない方も参加するようなので、このPRのような予防線のPRを 入れておくのもありかとは思います。

仕事のプロジェクトで開発しているアプリなら、このPRのような PRは意味がないですし、必要ないですね、確かに。