-
Notifications
You must be signed in to change notification settings - Fork 131
refactor(ci): 撲滅可能なUUOCを撲滅する #1081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
なのですが、一点だけ確認まで!
bashに不慣れなコントリビューターにとって読みづらい(=貢献しづらい)コードになりますが、その辺りの方針は考えていたりしますか? 👀
個人的には、この変更は理想なshellに近づく一方で、不慣れな人にわかりづらくなるという一長一短かもと感じました。
記号はググれないので、何をしているのか調べることすらできないこともありえるかなと。
ここのトレードオフをどう捉えるか、コアのメンテナとして方針、もしくはその途中の考えを言語化しておくと良いのかなーと!
(たぶん正解はなさそう。)
理由を挙げるとするならこんな感じでしょうか。
まあただ |
あ、言語化しておいた方がいいのは利点ではなく、利点と欠点のバランスをどうするのかというトレードオフの観点かなと・・・! それとも欠点はないという感じですかね・・・? あるいは、そもそもこのPRによって何を解決したいのかがわからないという話なのかもです! 追記:もしかしてこれは課題提起・・・?(Issue・・・?) |
Bashを学んだ方法によっては
ですね。ただissueと比べてPRの場合、強い理由が無い限り棄却しにくいというのも確かにあると思います。今回はその視点が抜けていました。 課題定義の理由としては、いつかどこかのPRで |
なるほどです! とりあえずどうすべきかは置いといて、意見をここに書いておくと良さそうですかね。
↑これはどうでしょう? あとは利点を考えて、そのバランスで決めるのが良いと思います! |
Bashの
上記以外、つまりBashの
たぶんそうで、議論の場を移すとしたら VOICEVOX/voicevox_project#70 の一部として議論することになりそうかなと思っています。 ただまあ、私としてはぶっちゃけ最悪 |
欠点は
あと調べても まあbashの
話していて思いましたが、このややこしさはどちらかというとシェルスクリプトをどう扱うかに起因していると感じました! ちょこちょこと考えていく必要がある気はするから、issue作るのはアリ・・・かも・・・?
リポジトリメンテナの判断で、片方に寄せた方が良さそうだとはを感じます! |
シェルスクリプトについては話し合った方がよいことが他にも結構色々ありそうなので、主軸をそれにするのはありだと思います。 例えば、環境変数じゃない変数は全部小文字にしてSC2154とかの恩恵を受けようみたいな話とかも前々からしてみたいと思っていました。 VOICEVOX/voicevox_project#70 のsub-issueとして「シェルスクリプトをどうするか」を作り、そのsub-issueとしてUUoCをどうするか(今このPRで話している内容)とかシェル変数の大文字小文字をどうするかみたいな話題をぶら下げるというのはどうでしょうか? [追記] VOICEVOX/voicevox_project#70 はそんなにシェルスクリプトに焦点を当ててないので、親子関係であるべきかというと確かに微妙な気がしますね。ただシェルスクリプトはほぼほぼGHAから呼ばれるので、GHAのworkflowの一部とも言えなくもないような気もします。 |
独立させた方が良いかなと思いました!
普通ならそれでも良さそうなんですけど、sub-issueの1つ1つが他の ま~ギリギリ1つのissueでなんとかなるかも!!Discussion使ってみるのもありかもです。どちらでも!! |
内容
撲滅可能なUUOC (Useless use of
cat
)を撲滅する。以下のcat
の利用は放置。cat <<EOF | while read -r line; do; …
cat
を抜こうとすると、ヒアドキュメントを後ろの方に持って来なければならないため。<(cat <<< "$variable")
CIによるチェックは入れない。というより簡単にできるかがわからない。VOICEVOX organizationにおける"uuoc"の検索結果に引っ掛かるよう、このPRをマージするにせよクローズするにせよ判例を作るのが目的。
関連 Issue
その他