Bullrich/unify jobs #3
Conversation
Test was singular, it should be plural
The matrix jobs are already depended by other jobs. We can simply remove their dependency
It looks a bit better than simply adding a print line statement
Let's unify everything at once
Added a shorter name to make it easier to understand
WalkthroughThe recent updates introduce new verification jobs to the CI/CD pipeline, enhancing the robustness of the development process. Specifically, these changes ensure that both migrations and tests are thoroughly checked for success before proceeding, running on the latest Ubuntu environment. This initiative enhances the reliability and integrity of the codebase by ensuring that key components such as database migrations and various tests (runtime, integration, and build-chain specifications) are validated. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| confirmMigrationsPassed: | ||
| runs-on: ubuntu-latest | ||
| name: All migrations passed | ||
| # If any new job gets added, be sure to add it to this list | ||
| needs: [check-migrations] | ||
| steps: | ||
| - run: echo '### Good job! All the migrations passed 🚀' >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
The addition of the confirmMigrationsPassed job is a good practice for centralizing the success confirmation of migration-related tests. However, consider adding a conditional step or job status check to explicitly handle the scenario where the check-migrations job fails. This would improve clarity and maintainability by explicitly documenting the intended behavior in failure scenarios.
| runs-on: ubuntu-latest | ||
| name: All tests passed | ||
| # If any new job gets added, be sure to add it to this list | ||
| needs: | ||
| - runtime-test | ||
| - integration-test | ||
| - build-chain-spec-generator | ||
| steps: | ||
| - run: echo '### Good job! All the tests passed 🚀' >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
The addition of the confirmTestPassed job is a solid approach to streamline the testing process by having a single job that confirms the success of all tests. Similar to the previous file, consider adding a conditional step or job status check to handle scenarios where any of the dependent jobs (runtime-test, integration-test, build-chain-spec-generator) fail. This would ensure that the workflow's behavior in failure scenarios is clear and maintainable.
Unified all the tests into having one final step at the end.
This step will only run if all the previous steps have ran. If any of these tests fail, it will fail.
All the status checks will still appear in the PR, so the user can see any failed case:
But, by adding this last test as the only requirement, the list to block the branch becomes significantly smaller.
Summary by CodeRabbit