-
Notifications
You must be signed in to change notification settings - Fork 120
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
ci: cargo-denyのadvisories
だけcronでの実行にする
#893
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.
ちょっと目的を整理しておけると!
目的次第ですが、どっちもcron実行・mainブランチで実行にしても良いのかも?
たしかcargo-denyをmainブランチでのテストにしてた理由は、ファイルを更新するのは大変なので、コミッターでなくメンテナが更新すれば良いようにしたかったからだった記憶。
mainブランチのテストはたまに落ちて良いという設計っぽみ。
今回のPRはadvisories
のみmainブランチから外してcronにし、ほかはmainブランチで動かす感じですよね。
提案としては、まあどっちもcron・mainブランチで動かすようにすればよいのかな、みたいな。
(だとしたらファイルを分けなくて済むので)
追記:
あいや違うか、advisories
だけはauditで最悪スルーしてリリースできるけど、ほかは無視するとリリースできないみたいな・・・?
だとしたらなぜファイルを分けてるかのメモコメントというか、各ファイルが何を担当しているのかわかるようにするのが良さそう!
(audit(監査)だとなぜadvisories
だけしているのかわからない)
どちらかというと、 一方
|
なぁるほどです。 ちなみに「advisories以外のcargo-deny」は結構難しいあのdeny.tomlの編集が不要・・・だったりはしないでしょうか。 ジャストアイデアでやるべきかはわかりませんが、「advisories以外のcargo-deny」をtest.ymlに移しちゃって、mainブランチ以外では動かないようにするのもあり・・・かも・・・? |
逆ですね…
個人的には |
なるほどです! エンジンの方はファイル数削減がうまくいったので思いついた次第でした。 |
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です!
コメントドキュメント足しの提案をしました!
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!!
Ⅰ箇所コメントしていますが、変更するしないはどちらでも良さそう!
メンテナンス性が上がって嬉しいです!
Refs: VOICEVOX#893 (comment) Co-authored-by: Hiroshiba <[email protected]>
↑ あ、 |
内容
cargo-deny
ワークフローをlicenses
とaudit
に分離し、後者を毎日のcron実行とする。関連 Issue
その他
RustSecへの登録によって「何もしていないのにCIが落ちた」ケースが最近増えてきたため、ENGINEのようにcronだけで動かす方が妥当かなと思った次第です。
(もしかしたらENGINEと同様、Discordへの送信にした方がよいかもしれない)