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

Broken hashira-web in typecheck phase caused by mergery didn't wait the CI result #1162

Closed
kachick opened this issue Feb 16, 2024 · 12 comments · Fixed by #1163
Closed

Broken hashira-web in typecheck phase caused by mergery didn't wait the CI result #1162

kachick opened this issue Feb 16, 2024 · 12 comments · Fixed by #1163
Labels

Comments

@kachick
Copy link
Collaborator

kachick commented Feb 16, 2024

I don't know why some PRs merged and others are not

merged: #1154
blocked: #1161

@pankona Could you check?

@pankona
Copy link
Owner

pankona commented Feb 16, 2024

Thank you for your reporting!

@pankona
Copy link
Owner

pankona commented Feb 16, 2024

全然わからん

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

mergrey自体にはCI待つような機能がなくてラベル即マージにいってるはずなので、必須チェックをリポジトリ設定側でしないといけないはずだけど現状入れてないのでは(required が表示されてない)

@pankona
Copy link
Owner

pankona commented Feb 17, 2024

Screenshot_20240217-112343

一応プロテクションは入れているつもりではある

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

ここ自分もよくわかってはないんだけど、 choose をしてない状態じゃないですか。 で、こんときって全選択扱いみたいにはならなかった気がするんだよな・・・(mobu はそれで選んだはず)

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

https://dev.classmethod.jp/articles/github-branch-protection-make-status-checks-mandatory/

これとか

で、ここで required をつけると、今度はその required がそもそも走らないような時には automerge が効かなくなるので、全部のPRで重要な check を走らせる必要がある。それでいいなら良いんだけど、それだと重たいCIとかを小さい変更に対しても必須で走らせる必要が出てくるから、個人的に大きくて堅いプロジェクト以外では避けるようにはしてます・・・

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

尚 action が死にだしたのは #1032 => https://github.com/pankona/hashira/actions/runs/6617058313/job/17972581260 で、4ヶ月前にstyled-components のメジャーバージョンアップがあったときからの様子

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

https://github.com/styled-components/styled-components/blob/22e8b7f233e12500a68be4268b1d79c5d7f2a661/.github/ISSUE_TEMPLATE.md?plain=1#L17
styled-components/styled-components#4062

styled-components が v6 から DefinitelyTyped 不要になったのにv5の type へ依存してる package.json なのでそのせいかと思ったんだけど、どっちかというとサードパーティの styled-normalize とやらが未だに v5 までしか対応してないのにstyled-components あげちゃってる ミスマッチしてるのが原因っぽい

https://github.com/sergeysova/styled-normalize/blob/1131a414d601d342e41c7558ed64d3bd9db1a854/README.md#L18

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

というのに加えて、 その後 vite 側でもエラーが増えた状態でマージされている。 v5 から v4 へ落とすと2つ解消する代わりに v4 へバックポートされていない vitejs/vite#15714 で死ぬ。わりとずたぼろである。

vite の typecheck は良く壊れるので skipLibCheck 入れるか、v4の中でも更に古いバージョンまで落とすなりで解消と言うか見てみないふりは出来そう

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

"skipLibCheck": true, にすると ↑の styled-components で起こってた問題ももみ消されてしまうので、やはり検知が更に遅れるだけという気はする・・・

@pankona
Copy link
Owner

pankona commented Feb 17, 2024

ずたぼろはともかく、CI とおらないままマージしちゃうのが良くないですね。mergery で良いのかも含めてやり方を再確認したさありますな...。
調査ありがとうございます!

@kachick
Copy link
Collaborator Author

kachick commented Feb 17, 2024

とりあえず #1163 へ現状解消PRを出しておきました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants