Conversation
Hiroshiba
left a comment
There was a problem hiding this comment.
方針LGTMです!!
実装はなんかミスあり得るかもだからAI君にレビューしてもらうと良いかも。
There was a problem hiding this comment.
Pull request overview
This PR modifies the CI workflow configuration to ensure comprehensive testing runs on the main branch. Previously, only manual workflow dispatches (and releases in build_and_deploy.yml) would execute the full test suite, which caused issues to be discovered late in the release cycle. Now, pushes to the main branch will also trigger complete CI execution, helping catch problems earlier.
Key changes:
- Updated
IS_SIMPLE_TESTlogic to exclude main branch from simple testing - Consistent implementation across both test and build/deploy workflows
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/test.yml |
Modified IS_SIMPLE_TEST to run full tests on main branch and workflow_dispatch events |
.github/workflows/build_and_deploy.yml |
Modified IS_SIMPLE_TEST to run full tests on main branch, release events, and workflow_dispatch events |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path: java_artifact | ||
|
|
||
| build_xcframework: | ||
| if: ${{ !(github.event_name != 'release' && github.event_name != 'workflow_dispatch') }} # !env.IS_SIMPLE_TEST と同じ |
There was a problem hiding this comment.
@qryxip
AI君によると、ここのコメントが実態と合ってないとのことでした!
挙動を合わせるならこっちも変える必要がありそう。
envで計算するとここのifで使えないので、config jobでIS_SIMPLE_TESTを計算するようにすると共通化できそう。
#1229 では`build_xcframework`と`build_java_package`が対象外になってし まったので、この二つも動くようにする。 #1229 (comment) このPRでは問題箇所の修正のみに留める。
内容
0.16.2のリリースのときにDiscordでも言ったが、現状の
can_skip_in_simple_testの考えかただと、一番壊れやすい部分がCIされずにリリース直前になって壊れていることが発覚するということが起こる。というか頻発している。そのためmainブランチ上でのCIは"simple_test"としないこととする。CI時間については、 #382 のようにワークフローを分割したりキャッシュを(もちらん適切に)効かせるといった方向で対処するべき。
関連 Issue
その他