-
Notifications
You must be signed in to change notification settings - Fork 200
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
整理: シェルスクリプトに lint を適用 #1122
整理: シェルスクリプトに lint を適用 #1122
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です!!
あっ タイトルがさすがに何かわからないのでもう少し詳しく書いていただけると助かるかもです!
内容の1行目に書いてあるシェルスクリプトの lint によるリファクタリング
とか!
@Hiroshiba |
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 です!! ありがとうございます!!!
@Hiroshiba |
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!!
提案コメント書きましたが、まあそんなに変わらないだろうということで、そのままマージします!
そもそもやっぱり、yamlファイル内でビルドコマンドを列挙するのが良くない気がしました。
こういうactionlint問題とかも生じちゃうのもありますが、そもそもローカルで動かすときに非常に苦労するので・・・。
まあそれはそれでいちいちbashファイルを見に行かないといけないので、一長一短ではあるのですが・・・。
コードジャンプができたら楽なんですけどね~。
内容
シェルスクリプトの lint によるリファクタリング
actionlint と ShellCheck が衝突し false alert を出す場合がある(actionlint docs)。
# Lint disabled because of actionlint's implicit ________________ replacement \n # shellcheck disable=SC2193
によりこれらを抑制した。また venv ファイルへのアクセスは lint 対象外とした。
また lint に従い変数表記を修正した。
関連 Issue
part of #525