-
Notifications
You must be signed in to change notification settings - Fork 3
Review of CI (github action workflows) #1723
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.
Pull Request Overview
Optimizes CI workflow triggers to reduce unnecessary builds and improve efficiency by removing redundant tests and adding manual dispatch capability to all workflows.
- Reduced workflow frequency by removing triggers for push events and limiting PR triggers to specific types
- Added path-based filters to prevent unit and integration tests from running when irrelevant files change
- Restructured the linter workflow to run super-linter in parallel and removed redundant pylint step
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/source/developer-guide/continuous_integration.md | Added comprehensive table documenting workflow triggers and conditions |
| .github/workflows/changelog.yml | Added manual dispatch and removed synchronize trigger |
| .github/workflows/build-simtools-dev-image.yml | Removed push and tag triggers to reduce unnecessary builds |
| .github/workflows/CI-unittests.yml | Added path filter to skip when integration tests change |
| .github/workflows/CI-linter.yml | Separated super-linter into parallel job and removed redundant pylint step |
| .github/workflows/CI-integrationtests.yml | Added path filter to skip when unit tests change |
| .github/workflows/CI-docs.yml | Changed trigger from synchronize to ready_for_review to reduce builds |
Comments suppressed due to low confidence (1)
docs/source/developer-guide/continuous_integration.md:1
- The path filter column appears to list paths that are ignored, but the column header doesn't clearly indicate this. Consider adding 'ignored' or 'paths-ignore' to the header to clarify that these paths exclude the workflow from running.
# Continuous Integration
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1723 +/- ##
=======================================
Coverage 98.14% 98.14%
=======================================
Files 88 88
Lines 11485 11485
=======================================
Hits 11272 11272
Misses 213 213 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
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.
Thanks for the PR!
|
Thanks @EshitaJoshi ! |





We have a lot of CI workflows running. Not all seem to be necessary, especially those running for each push.
This PR reviews all CI workflows and optimizes the triggers. Also added a table to the documentation with the different triggers.
Schedule changes:
workflow_dispatchto all workflows to allow for manual executionbuild-simtools-dev-image: there is no test running with this workflow (was different a long time ago). All what is tested is that we can install simtools on a linux image; this is done for each integration test; so not needed to run on pull tests.). Run it for changes to `docker/Dockerfile-devv*CI-docs: runs on PR [opened, ready_for_review]and notsynchronize` anymore (meaning for each push)Functionality
pylintstep inCI-linter, aspylintis included inpre-commit(it wasn’t a while ago)super-linterstep to run now in parallel for improved efficiency