Voicevox org内で共通のWorkflowのスタイルガイドを定めたい + それのLinterを作りたい

[sevenc-nanashi]

内容

Voicevox org内で共通のWorkflowガイドライン定め、それのLint actionを作りたいです。

Pros 良くなる点

例えばShellCheck/actionlint/prettierなどのツールのかけ忘れやEngine内で行っているnameのプレフィックスの普及などができる

Cons 悪くなる点

メンテコスト

実現方法

merge-gatekeeperみたいに- uses: voicevox/workflow-lint@mainみたいなのを全リポジトリのlintに追加すればよさそう。

その他

（なし）

[sevenc-nanashi]

とりあえず草案：

• Prettierをかける
• actionlint + shellcheckをかける（shellcheck君は思想が強いので本当に必要かどうかは要検討）
• workflow名、job名、step名のケースを統一する（kebab-caseとsnake_caseのどちらにするかは要検討）
• job名はbuild/test/lint/checkのような一般名詞またはそれのあとに何か加えたもの（test-hogeなど）にする（判定はホワイトリスト式になりそう）
• step名に<Setup>のような何をやっているかを書く
• そもそもstep.nameを必須にしてもいいかも
• pull_request_targetで発火する場合はworkflow名をDANGEROUS_から始める
• メジャーバージョンは最新にする（これが本当にいるかは微妙かも）

[sevenc-nanashi]

とりあえずメンテナ勢とここら辺に詳しそうな方をメンション： @Hiroshiba @qryxip @tarepan

[Hiroshiba]

ちょっとタイトルがややこしいかも、VOICEVOX Organization共通のGithub Workflowファイル共通のスタイルガイドを定めたい、って感じですかね？
（Workflowルールだと、運用ルールとかともかぶりそうなので、フォーマットのことだと分かりやすいようにしておくのが良いかなって提案です！）

[Hiroshiba]

個人的には共通ルールの導入も、静的解析Github Actionsの作成も賛成です！
とりあえずこれを導入しときゃいい、ってのがあると便利だと思います。

草案に関して意見あるところだけコメントしてみました！　コメントしてないところは賛成です。

• Prettierをかける
  • jsやtsやyamlやjsonにもかけれるので、対象とするファイルを認識合わせておくと良さそう？
• actionlint + shellcheckをかける（shellcheck君は思想が強いので本当に必要かどうかは要検討）
  • shellcheck君はオフにできるようにしておくと一旦良いかも
• workflow名、job名、step名のケースを統一する（kebab-caseとsnake_caseのどちらにするかは要検討）
  • step名はケース無さそう？　多分英語の場合スペース区切り。
  • 全部リポジトリでケースを統一しなくても良いかもと思ってましたけど、別に統一しちゃってもよさそう。
  • たぶん多数決を取るとスネークケースになるはず。
• job名はbuild/test/lint/checkのような一般名詞またはそれのあとに何か加えたもの（test-hogeなど）にする（判定はホワイトリスト式になりそう）
  • lintとcheckはtestに集約できそう感
  • ENGINEはbuildとtestのみで、意外とそれで回ってそう
  • 例外として感がておくと良さそうなのは例えばrelease_latest_dev.yml・labeler.yml・trigger_preview_pages.yml・merge_gatekeeper.ymlとかですかね
• step名に<Setup>のような何をやっているかを書く
  • これもホワイトリスト型にする手もあるけど、しばらくは追加がありえるかもだから難しいかもですね
  • 例えばどういうものがあるかのリストアップをしておくと良さそう
  • 割と今のエンジンに導入されてるやつだけでなんとかなる感もある
• そもそもstep.nameを必須にしてもいいかも
  • ありかも。まあでもちょっと煩雑かも。難しいところ。
• メジャーバージョンは最新にする（これが本当にいるかは微妙かも）
  • 確かにあると便利そうだけど、メジャーバージョンがリリースされるたびにエラーになるのはちょっと厄介かも。
  • これはrenovateとかに任せる手もありそう。

[Hiroshiba]

別の観点ですが、静的解析ツールに関して適用するルールをホワイトリスト形式にするのかブラックリスト形式にするのか少し迷いました。
半自動的にアップデートされてほしいので、不要なルールをブロックする形が良さそうな予感がしています！

各リポジトリに導入する時、バージョン固定するかどうかも迷いどころに思いました。
静的解析側がアップデートされた後にpushされたものがエラーになるよなーと。
かと言ってバージョン固定すると、いろんなリポジトリでアップデートしていくのが面倒。

まあ･･･どちらかに倒すなら、各リポジトリでバージョン固定するのが丸そうですかなぁ。
たぶん各リポジトリは最初に導入する時のメリットが大半で、あとは好きなタイミングでアップデートしていけば良さそう感！

[Hiroshiba]

追加の草案：

• １行目にそのworkflowファイルの説明を必須にする

[sevenc-nanashi]

• Prettierをかける
  • jsやtsやyamlやjsonにもかけれるので、対象とするファイルを認識合わせておくと良さそう？

.github/workflows下、あと.github/actions下ですね。そういえば.github/actions下のファイル名のケースも決めたほうがよさそう。

• actionlint + shellcheckをかける（shellcheck君は思想が強いので本当に必要かどうかは要検討）
  • shellcheck君はオフにできるようにしておくと一旦良いかも

ですねぇ。とは思ったけどほぼ全部のリポジトリでshellcheck君が無効化される可能性もありそう

• workflow名、job名、step名のケースを統一する（kebab-caseとsnake_caseのどちらにするかは要検討）
  • step名はケース無さそう？　多分英語の場合スペース区切り。

あ、step.idですね。後でsteps.hoge.outputs.fugaみたいなときに使うやつ。

• 全部リポジトリでケースを統一しなくても良いかもと思ってましたけど、別に統一しちゃってもよさそう。
• たぶん多数決を取るとスネークケースになるはず。

ですねー。

• job名はbuild/test/lint/checkのような一般名詞またはそれのあとに何か加えたもの（test-hogeなど）にする（判定はホワイトリスト式になりそう）
  • lintとcheckはtestに集約できそう感
  • ENGINEはbuildとtestのみで、意外とそれで回ってそう
  • 例外として考えておくと良さそうなのは例えばrelease_latest_dev.yml・labeler.yml・trigger_preview_pages.yml・merge_gatekeeper.ymlとかですかね

• release_latest_dev.yml、trigger_preview_pages.yml：deployとかですかね？
• merge_gatekeeper.yml、labeler.yml：多分「リポジトリ管理補佐」みたいな枠が必要かも。maintain？

• step名に<Setup>のような何をやっているかを書く
  • これもホワイトリスト型にする手もあるけど、しばらくは追加がありえるかもだから難しいかもですね
  • 例えばどういうものがあるかのリストアップをしておくと良さそう
  • 割と今のエンジンに導入されてるやつだけでなんとかなる感もある

前処理か本処理か、で別れてきそう。job名の一般名詞 + Setupとか？

• そもそもstep.nameを必須にしてもいいかも
  • ありかも。まあでもちょっと煩雑かも。難しいところ。

省略したらstep名のlintで引っ掛かりそうな気がするので、step名lintを実装するとnameの必須化も自動的に実装されそうな気がしました。

• メジャーバージョンは最新にする（これが本当にいるかは微妙かも）
  • 確かにあると便利そうだけど、メジャーバージョンがリリースされるたびにエラーになるのはちょっと厄介かも。
  • これはrenovateとかに任せる手もありそう。

確かに。 >renovate
Voicevox org共通のrenovateルールとかも決めたい気持ちがありますね（やるなら別Issueで）

[Hiroshiba]

deployとかですかね？

こうするとbuildとdeployが両方あるファイルをどうするかが困るんですよねぇ。。。

多分「リポジトリ管理補佐」みたいな枠が必要かも。maintain？

なるほどです！！　確かにあるといいのかも。
この名前で良いのか、一旦全VOICEVOXリポジトリ内のworkflowsファイル見て、そういう名前つけても良さそうか把握しておくと良さそう？
（後で変えるのすごい面倒なので。。。。）

• step名に<Setup>のような何をやっているかを書く
  • これもホワイトリスト型にする手もあるけど、しばらくは追加がありえるかもだから難しいかもですね
  • 例えばどういうものがあるかのリストアップをしておくと良さそう
  • 割と今のエンジンに導入されてるやつだけでなんとかなる感もある

前処理か本処理か、で別れてきそう。job名の一般名詞 + Setupとか？

ENGINEのはすでに考えられたものなので、一旦それをリストアップしてみて、無理がありそうなのやよく知ってるリポジトリのステップで当てはめが難しいのを探してみるとかすると良さそう！
ちなみにSetup・Build・Deploy・Testの４つでした。<Build/Deploy>ってのもあった。

• そもそもstep.nameを必須にしてもいいかも
  • ありかも。まあでもちょっと煩雑かも。難しいところ。

省略したらstep名のlintで引っ掛かりそうな気がするので、step名lintを実装するとnameの必須化も自動的に実装されそうな気がしました。

確かに。まあ省略されてたら無視する手もありそうだけど。
まあでも結構自明なのもありそうなんですよね～～～～。
特にプロトタイプを作ってる時とかはcheckoutとかのnameを省略できないのが煩わしく感じるかも、くらいの気持ち！

[qryxip]

kebab-caseとsnake_case

ちょっとこれについて私見を述べると、workflow file名もjob名もstep名もinput名も、kebab-caseの方が一般的な認識です。github.com/actionsとかgithub.com/githubとかを見てもそうなんじゃないかと思います。

GHAの構文自体の各キーワードもsnake_caseとkebab-caseが混在していますが、kebab-caseの方が優勢だった気がしています。

[Hiroshiba]

kebab-caseとsnake_case

ちょっとこれについて私見を述べると、workflow file名もjob名もstep名もinput名も、kebab-caseの方が一般的な認識です。github.com/actionsとかgithub.com/githubとかを見てもそうなんじゃないかと思います。

これ調べてみました。
github actionsのdiscussionでも上がってるけど公式な解答はなし。
actionlintは「ドキュメント内に記載がない。統一できるのは良いことだと思う。検討する。」という状況っぽみ。
runnerのissueで、POSIXだと環境変数にハイフン使えないからinputsの名前を自動変換する提案があったけど、影響が広いから見送られた模様。

結論はたぶん、snake caseじゃないとダメな場面はあるけど、慣習的にはcebab caseになってる。
環境変数にしうるのでsnake caseにする、というのはわりと飲めそうな言い方だなーと思いました。