-
Notifications
You must be signed in to change notification settings - Fork 31
Simplify CI workflow and actions #1017
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
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.
Code Review
This pull request simplifies the CI workflow by refactoring the ci-checks action to handle a single check at a time instead of a list of checks. This removes the need for JSON parsing and simplifies the conditional logic for running steps. The consolidation of tests-* jobs into a single dynamic step is a good improvement. However, I've found a critical issue: a recurring typo in the action file where input.check is used instead of inputs.check. This will prevent most of the checks from running. Please see my detailed comment for a suggested fix.
1da223d to
e47781a
Compare
|
/gemini review |
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.
Code Review
This pull request simplifies the ci-checks GitHub Action by changing its input from a list of checks to a single check. This significantly improves the readability and maintainability of the workflow by replacing complex contains(fromJSON(...)) expressions with simple string comparisons. The consolidation of the sharded test steps into a single dynamic step is a particularly nice improvement. The related ci-footprint action is also simplified. Overall, these are great changes. I have one minor suggestion to make the test sharding logic even cleaner by using a built-in GitHub Actions function.
|
/gemini review |
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.
Code Review
This pull request simplifies the CI workflow by refactoring the ci-checks action to handle a single check per invocation instead of a list of checks. This is a great improvement that will likely allow for better parallelization and clearer job definitions in the calling workflows. The removal of the use-cache input and related logic in both ci-checks and ci-footprint actions further streamlines the implementation. The changes are well-executed and improve the maintainability of the CI configuration. I have one minor suggestion to improve the readability of a condition.
b188793 to
2094515
Compare
We don't run all checks when building the cache anymore.
We don't run all checks when building the cache anymore.