Skip to content
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

mainのCIが失敗している #42

Closed
kobkaz opened this issue Jan 15, 2024 · 13 comments
Closed

mainのCIが失敗している #42

kobkaz opened this issue Jan 15, 2024 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@kobkaz
Copy link
Contributor

kobkaz commented Jan 15, 2024

https://github.com/arkedge/c2a-tlmcmddb/actions/runs/7523751359

@sksat が詳しいですか?

@kobkaz kobkaz self-assigned this Jan 15, 2024
@kobkaz kobkaz added the bug Something isn't working label Jan 15, 2024
@sksat
Copy link
Member

sksat commented Jan 15, 2024

あー,これ sksat/action-clippyon: push で発火して,clippy の warning がある時にたまに起こる問題ですね......
そもそもログが分かりにくいなどもあるのでどうにかします.

@sksat sksat assigned sksat and unassigned kobkaz Jan 15, 2024
@kobkaz
Copy link
Contributor Author

kobkaz commented Jan 15, 2024

clippyのwarningは #41 で新たに発生したようなのですが、これにreviewdogが何も言っていないのも不具合でしょうか?

@sksat
Copy link
Member

sksat commented Jan 15, 2024

こっちは不具合というか workflow 側の設定不備ですね.今は on: push 時に Pull Request モードで reviewdog が動いているので.

@sksat
Copy link
Member

sksat commented Jan 15, 2024

あ,PR 上で言ってきてない方か.それはおかしそう.

@sksat
Copy link
Member

sksat commented Jan 15, 2024

@kobkaz sksat/action-clippy v0.5.0 で対応しました.Broken pipe はちゃんと調べてみたら Rust 側の問題でした.
https://github.com/sksat/action-clippy/releases/tag/v0.5.0

@sksat
Copy link
Member

sksat commented Jan 15, 2024

あー, #41Field を書き換えてこの型サイズが大きくなった結果それを使っている SubEntry に warning が出た,という形なので added filter だと捕まえられなかった,ということっぽいですね.

@sksat
Copy link
Member

sksat commented Jan 15, 2024

sksat/action-clippy 側でデフォルトの filter_mode を diff_context, file あたりに変えるのはアリだけど,それでも取りこぼすことはあるだろうし,悩ましいな.結局プロジェクト毎に気にするやつは nofilter を明示的に指定する,とかかも.

@sksat
Copy link
Member

sksat commented Feb 8, 2024

そもそも clippy は行単位の lint してるわけじゃないんだからデフォルト設定は nofilter でいいでしょ,ということにした
https://github.com/sksat/action-clippy/releases/tag/v0.7.0

@sksat
Copy link
Member

sksat commented Feb 8, 2024

それはそれとして, #43 (と #44 )で「何故か(何も言わずに)CI がコケている」という事象は解決している.今は clippy の warning がちゃんと出ているので,そっちの中身の修正は @kobkaz にお任せした方がよいかな.

@sksat
Copy link
Member

sksat commented Feb 8, 2024

あー,でもこれはどちらかというと PR 上では status check が通っている(annotation は出ている)けれど,マージしたら main ではコケる,というところに驚きがあるのか.fail_on_error: true するか,main での clippy warning は表示だけして素通りするか,という方が大事か.

@sksat
Copy link
Member

sksat commented Feb 8, 2024

↑は #48 で修正した

@sksat
Copy link
Member

sksat commented Feb 8, 2024

それはそれとして,この挙動だと驚きは少ないけどマージ済みの clippy warning は見逃してしまうかも.なのでこの Issue は中身を修正してから閉じたいのと,fail_on_error: true するかどうかを決めたい.

@sksat
Copy link
Member

sksat commented Feb 8, 2024

個人的には clippy の言うことにはできるだけ従いたいので fail_on_error: true 寄りかな.あえて無視したいケースは十分に少ないし,無視する書き方もできるし.sksat/action-clippy のデフォルト設定も変えていいかもしれない.

問題があるとすれば「知らんうちに新しい lint が出てきた」というやつで,これはちょっと悩ましい.
個人的には Rust version を固定(clippy version を固定 / rust-toolchain する)すればいいやんけ,とは思いつつ,毎回やるにはちょっと strict すぎる要求だとも思わなくはない(ex: 雑に stable にしておきたい).

@kobkaz kobkaz closed this as completed Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants