-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: 🔧 setup commitlint to enforce conventional commits #2153
Conversation
Important Review skippedReview was skipped due to path filters Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughWalkthroughThe changes introduce commit message linting and testing into the Git commit process through the addition of Husky hooks. A new configuration file for commitlint is created to enforce standardized commit message formats. Modifications to the Changes
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Here's the code health analysis summary for commits Analysis Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## dev #2153 +/- ##
=======================================
Coverage 28.97% 28.97%
=======================================
Files 224 224
Lines 12443 12443
Branches 486 473 -13
=======================================
Hits 3605 3605
Misses 8838 8838
☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
package.json (1)
110-112
: Consider updating the lint-staged configuration to include TypeScript files.The addition of lint-staged and its configuration to run ESLint on staged JavaScript files is a good practice to ensure that only properly linted code is committed.
However, the project seems to be using TypeScript (based on the presence of TypeScript-related dependencies and configurations). Consider updating the lint-staged configuration to include TypeScript files as well:
"lint-staged": { - "**/*.js": "eslint" + "**/*.{js,ts}": "eslint" },This way, both JavaScript and TypeScript files will be linted before committing.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (3)
- .husky/commit-msg (1 hunks)
- commitlint.config.js (1 hunks)
- package.json (1 hunks)
Files skipped from review due to trivial changes (1)
- commitlint.config.js
Additional comments not posted (5)
.husky/commit-msg (1)
1-1
: Great addition to enforce commit message conventions!This change integrates
commitlint
into the Git commit process, ensuring that commit messages adhere to the defined rules before allowing the commit to proceed. It will help maintain a consistent and informative commit history.Please ensure that all contributors are aware of this change and update their commit message practices accordingly. Consider providing documentation or examples of the enforced commit message conventions to facilitate the transition.
package.json (4)
23-23
: LGTM!The addition of the "prepare" script with the value "husky" is a good practice for setting up pre-commit hooks using Husky. This aligns with the PR objective of enforcing conventional commits using commitlint.
113-117
: LGTM!The addition of the Husky configuration with a pre-commit hook that runs lint-staged is a good practice. This ensures that the staged files are linted before they are committed, helping maintain code quality.
The combination of Husky, lint-staged, and commitlint provides a solid setup for enforcing code quality and commit message conventions, which aligns with the PR objectives.
103-108
: LGTM!The addition of the "exclude" configuration in the "c8" section is a good practice. Excluding directories like "coverage", "docs", and "docs2", as well as test files, from the code coverage reports helps focus on the coverage of the actual source code.
This configuration enhances the precision of the code coverage metrics and aligns with common practices.
66-67
: Verify the commitlint configuration.The addition of the "@commitlint/cli" and "@commitlint/config-conventional" dependencies is necessary for setting up commitlint to enforce conventional commits, which aligns with the PR objectives.
However, a configuration file for commitlint (e.g., commitlint.config.js) is not visible in the provided code. Please ensure that the commitlint configuration file exists and is properly set up.
Run the following script to verify the commitlint configuration:
Verification successful
Commitlint configuration verified and correct.
The commitlint configuration has been successfully verified. A
commitlint.config.js
file exists in the project root with the following content:export default { extends: ['@commitlint/config-conventional'] };This configuration correctly extends the '@commitlint/config-conventional' preset, which aligns with the added dependencies in package.json. The setup is appropriate for enforcing conventional commits as intended.
No further action is required regarding the commitlint configuration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and content of the commitlint configuration file. # Test: Check if the commitlint configuration file exists. # Expect: The file to exist (e.g., commitlint.config.js, .commitlintrc.js, .commitlintrc.json, etc.). fd --type file --regex '^\.?commitlint(rc)?\.(js|json|ya?ml)$' # Test: If the file exists, check if it extends the conventional preset. # Expect: The configuration to extend "@commitlint/config-conventional". fd --type file --regex '^\.?commitlint(rc)?\.(js|json|ya?ml)$' --exec cat | rg $'@commitlint/config-conventional'Length of output: 177
Script:
#!/bin/bash # Description: Further investigation of commitlint configuration # Test: Check if commitlint configuration is defined in package.json echo "Checking package.json for commitlint configuration:" cat package.json | jq '.commitlint' # Test: Search for any files mentioning commitlint echo -e "\nSearching for files mentioning commitlint:" rg -i commitlint # Test: Check for any JavaScript files that might contain commitlint configuration echo -e "\nChecking JavaScript files for commitlint configuration:" rg -t js 'module.exports\s*=\s*{[\s\S]*commitlint[\s\S]*}' -A 5 -B 5 # Test: Look for any npm scripts related to commitlint echo -e "\nChecking for npm scripts related to commitlint:" cat package.json | jq '.scripts | to_entries[] | select(.value | contains("commitlint"))'Length of output: 4252
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
.husky/pre-commit (1)
1-1
: Consider the impact on the development workflow.Running tests before each commit is a good practice to maintain code quality and catch errors early. It encourages developers to write tests and keep them passing.
However, running the full test suite on every commit might slow down the development process, especially for large projects with extensive tests. Consider the impact on the workflow and the trade-off between the added quality check and the potential slowdown.
If the impact on the workflow is significant, you could explore alternative approaches, such as:
- Running only a subset of tests that are relevant to the changes made in the commit.
- Running tests only on certain branches (e.g., main, develop) or before merging pull requests.
- Providing an option to skip the pre-commit hook when needed (e.g., using
git commit --no-verify
).These alternatives can strike a balance between ensuring code quality and maintaining a smooth development workflow. Let me know if you would like me to provide examples of how to implement any of these approaches.
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/pg-hstore@2.3.4, npm/pg@8.13.0, npm/pino-pretty@11.2.2, npm/pino@9.4.0, npm/prettier-eslint@16.2.0, npm/prettier@3.3.3, npm/rimraf@6.0.1 |
Quality Gate passedIssues Measures |
Summary by CodeRabbit
New Features
Enhancements
These changes aim to streamline development processes and enhance collaboration within the team.