-
-
Notifications
You must be signed in to change notification settings - Fork 653
chore(ci): modernize #4620
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
base: main
Are you sure you want to change the base?
chore(ci): modernize #4620
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
This PR modernizes the CI infrastructure by replacing commitlint with GitHub Actions-based PR title validation and consolidating CI workflows. Key changes include:
- Removal of commitlint dependencies and local commit message validation
- Migration from
nodejs.ymlto a simplifiedci.ymlworkflow - Addition of automated PR title validation using GitHub Actions
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Removes @commitlint/cli and @commitlint/config-conventional dependencies |
| package-lock.json | Removes commitlint and related dependency entries; also removes "peer" flags from multiple packages |
| commitlint.config.js | Deletes commitlint configuration file |
| .husky/commit-msg | Removes husky hook for commit message validation |
| .github/workflows/validate-pr-title.yml | Adds new workflow for PR title validation using amannn/action-semantic-pull-request |
| .github/workflows/nodejs.yml | Removes old CI workflow |
| .github/workflows/ci.yml | Creates new consolidated CI workflow with lint and test jobs |
| .github/workflows/dependency-review.yml | Updates workflow name and step descriptions |
| .github/dependabot.yml | Changes GitHub Actions update schedule from daily to monthly and adds grouping |
| .cspell.json | Adds "amannn" and removes "commitlint" from dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4620 +/- ##
=======================================
Coverage 89.72% 89.72%
=======================================
Files 9 9
Lines 1401 1401
Branches 448 448
=======================================
Hits 1257 1257
Misses 144 144 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@avivkeller I make some valiuable change in our CI from your PR here - #4623, feel free to rebase and improve more if you have something, I prefer to use matrix everywhere because it is using variable and less hard coded values |
|
Perhaps it's a personal preference, but I find redundant matrices to be a bad practice, since they are meant to run multiple variations, not single ones. |
|
@avivkeller We use this value in two places - in run-on and node-version, we've previously encountered problems where a contributors forgot to update both (and more than both), so we decided to use a matrix because it allows you to change the value in only one place and not forget about the others |
|
Anyway in future we are planning to reuse workflows from |
Modernize the CI with improvements for brevity and speed
The Required Checks
CI / LintCI / SmoketestsCI / Test - macos-latest (Node.js 18.x)CI / Test - macos-latest (Node.js 20.x)CI / Test - macos-latest (Node.js 22.x)CI / Test - macos-latest (Node.js 24.x)CI / Test - ubuntu-latest (Node.js 18.x)CI / Test - ubuntu-latest (Node.js 20.x)CI / Test - ubuntu-latest (Node.js 22.x)CI / Test - ubuntu-latest (Node.js 24.x)CI / Test - windows-latest (Node.js 18.x)CI / Test - windows-latest (Node.js 20.x)CI / Test - windows-latest (Node.js 22.x)CI / Test - windows-latest (Node.js 24.x)Validate PR / Lint Commit Messages