ci: split configs #276
ci: split configs #276
Conversation
upgrade pre-commit frameworks
WalkthroughThe pull request introduces several configuration files and updates existing ones to enhance the development workflow for a TypeScript project. Key changes include the addition of Babel and ESLint configurations, commit linting rules, and a Mocha testing setup. The project also sees updates to versioning information in JSON files, as well as enhancements in 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
.nycrc.yml (1)
5-7: Consider adding HTML reporter for local developmentThe current reporters (
lcovandtext) are good for CI integration and command-line feedback. However, adding the HTML reporter could enhance local development by providing detailed, navigable coverage reports.reporter: - lcov - text + - html.lintstagedrc.yml (2)
3-5: Consider optimizing the test execution strategy.Running the full test suite (
npm test) on every TypeScript file change might significantly slow down commits, especially in larger codebases. Consider running only affected tests or moving full test runs to pre-push hooks.'src/*.ts': - eslint --cache --fix - - npm test + - npm test -- --findRelatedTests
6-9: Reconsider package.json processing frequency.Running
npm dedupeon every commit might be excessive and could slow down the commit process. Consider moving this to a less frequent hook, such as pre-push or post-merge.package*.json: - prettier --write - - npm dedupe - npx sort-package-json.releaserc.yml (1)
6-13: Simplify asset management configuration.The
CHANGELOG.mdfile is listed as an asset in both the changelog and git plugins. While this works, it's redundant. The git plugin configuration is sufficient.- - '@semantic-release/changelog' - - assets: - - CHANGELOG.md - - '@semantic-release/git' - assets: - package.json - package-lock.json - CHANGELOG.md
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.babelrc.json(1 hunks).commitlintrc.ts(1 hunks).eslintignore(1 hunks).eslintirc.yml(1 hunks).husky/commit-msg(1 hunks).husky/pre-commit(1 hunks).lintstagedrc.yml(1 hunks).mocharc.yml(1 hunks).nycrc.yml(1 hunks).releaserc.yml(1 hunks)data/hash.json(1 hunks)data/version.json(1 hunks)data/version.txt(1 hunks)package.json(1 hunks)tsup.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- .husky/pre-commit
- .eslintirc.yml
- data/version.txt
- .eslintignore
- data/version.json
- .babelrc.json
- tsup.config.ts
- .commitlintrc.ts
- .mocharc.yml
- data/hash.json
🔇 Additional comments (9)
.nycrc.yml (3)
1-3: LGTM! Exclusion patterns follow best practices
The exclusion patterns correctly ignore test files and build output from coverage reporting.
4-4: Verify if skipping full coverage reports is intended
Setting skip-full: true will omit full source code reporting from coverage output. While this makes reports more concise, it might hide useful information for debugging coverage issues.
✅ Verification successful
Skip-full setting aligns with the project's coverage reporting workflow
The skip-full: true setting is appropriate for this project because:
- Coverage reporting is primarily used with Coveralls (
nyc report --reporter=text-lcov | coveralls) - The project uses both local test coverage (
nyc mocha) and CI coverage reporting - Text-lcov reporter is used, which is designed for machine consumption rather than human reading, making full source reporting unnecessary
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this setting aligns with existing coverage reporting practices
# Look for previous NYC configurations
fd -t f -e json -e yml -e yaml -e js -e ts '.nycrc'
# Check CI workflow files for coverage reporting steps
fd -t f -g '*workflow*.{yml,yaml}' .github/workflows/
Length of output: 107
Script:
#!/bin/bash
# Let's try a broader search for coverage configuration and CI files
# Search for any NYC config files with different naming patterns
fd -t f -g '*nyc*'
# Search for coverage-related configurations in package.json
rg -l "nyc" package.json
# Look for coverage reporting in workflow files with a broader pattern
fd -t f workflow .github/
# Search for coverage-related commands in package.json scripts
rg '"scripts"' -A 10 package.json
Length of output: 558
1-7: Verify NYC configuration compatibility with Mocha setup
Let's ensure this NYC configuration properly integrates with the project's Mocha test setup mentioned in the summary.
✅ Verification successful
Let me gather more information about the Mocha configuration and test setup.
Let me check the actual test file content and NYC setup to make a final verification.
NYC configuration is properly aligned with the project setup
The NYC configuration is correctly set up and compatible with the project's Mocha test environment:
- The
excludepatterns correctly targetsrc/spec/**where test files are located - The reporters (lcov and text) align with the coverage script in package.json
- The configuration works with TypeScript setup as evidenced by tsconfig.json and the existing test file
- Test patterns and NYC integration are properly configured in package.json scripts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential configuration conflicts
# Look for Mocha configuration
fd -t f -e yml -e yaml -e js -e ts '.mocharc'
# Check if test files follow the excluded pattern
fd -t f -g '*.spec.ts' src/spec/
# Verify package.json test scripts
rg -A 5 '"test":|"coverage":' package.json
Length of output: 568
Script:
#!/bin/bash
# Check Mocha configuration and test patterns
# Look for mocha-specific configurations in package.json
rg '"mocha":' -A 10 package.json
# Check test file patterns and structure
fd -t f -g '*.spec.*' -g '*.test.*'
# Look for test-related imports and configurations
rg -g '*.spec.*' -g '*.test.*' 'describe|it|before|after'
Length of output: 609
Script:
#!/bin/bash
# Check test setup and NYC integration
# Check the test file content
cat src/spec/Build.spec.ts
# Look for any tsconfig or mocha configuration that might affect the test setup
fd -t f tsconfig.json
cat tsconfig.json
# Check if there are any other test files that might be affected
fd -t f -g '*spec*' -g '*test*' --exclude 'node_modules'
Length of output: 884
.husky/commit-msg (1)
1-1: LGTM! Verify commitlint installation.
The commit-msg hook is correctly implemented. The --no flag is a good practice to prevent automatic package installation.
Let's verify the commitlint setup:
✅ Verification successful
Let me gather more information about the commitlint configuration.
Let me check the content of the commitlint configuration file to ensure it's properly set up.
Commitlint setup is properly configured
The commit-msg hook is well-implemented and all required commitlint packages are correctly installed as dev dependencies:
@commitlint/cli(^19.6.0)@commitlint/config-conventional(^19.6.0)@commitlint/types(^19.5.0)
The .commitlintrc.ts configuration extends the conventional commit standard with custom rules for line length limits.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if commitlint and its configuration are properly set up
if [ -f "package.json" ]; then
echo "Checking commitlint dependencies..."
cat package.json | jq '.dependencies["@commitlint/cli"], .devDependencies["@commitlint/cli"]'
echo "Checking for commitlint config..."
fd -e js -e json -e yml -e yaml -e ts "commitlint" --exec cat
fi
Length of output: 356
Script:
#!/bin/bash
# Check for commitlint config in common locations and package.json config
echo "Checking package.json for commitlint config..."
cat package.json | jq '.commitlint'
echo -e "\nChecking for commitlint config files..."
fd '(commitlint|.commitlintrc)' --hidden --no-ignore
echo -e "\nChecking for extends in package.json..."
cat package.json | jq '.dependencies["@commitlint/config-conventional"], .devDependencies["@commitlint/config-conventional"]'
Length of output: 603
Script:
#!/bin/bash
# Check content of .commitlintrc.ts
cat .commitlintrc.ts
echo -e "\nVerifying all required commitlint packages..."
cat package.json | jq '{
dependencies: [.dependencies | to_entries[] | select(.key | startswith("@commitlint"))],
devDependencies: [.devDependencies | to_entries[] | select(.key | startswith("@commitlint"))]
}'
Length of output: 899
.releaserc.yml (1)
14-14: Verify release branch strategy.
Using development as the release branch is unusual. Typically, releases are made from main or master branches. Please confirm if this is intentional.
✅ Verification successful
Release branch strategy aligns with repository configuration
The repository is intentionally configured to use development as both the default branch and the release branch. This is evidenced by:
- The default branch is set to
development - Recent releases (v2.0.16 through v2.0.18) have been successfully published
- The semantic-release configuration correctly aligns with this setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check current branch setup and recent releases
if [ -f "package.json" ]; then
echo "Checking default branch..."
gh repo view --json defaultBranchRef --jq .defaultBranchRef.name
echo "Checking recent releases..."
gh release list --limit 3
fi
Length of output: 380
package.json (4)
5-18: LGTM! Comprehensive package metadata added
The added metadata fields improve package discoverability and provide essential project information. All URLs are correctly configured.
23-29: LGTM! Proper TypeScript distribution configuration
The files array and entry points are correctly configured for both ESM and CommonJS consumption.
68-70: Verify security implications of package overrides
The overrides section typically indicates security fixes. Let's verify these versions address known vulnerabilities.
✅ Verification successful
Package overrides address known security vulnerabilities
The specified overrides are appropriate security measures:
cross-spawn@^7.0.4: Overriding to version 7.0.4 or higher is secure as it's below the vulnerable range (7.0.0 to <7.0.5) identified in the latest ReDoS vulnerability from November 2024tough-cookie@^4.1.3: This version requirement matches exactly with the first patched version (4.1.3) that fixes the prototype pollution vulnerability from July 2023
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for security advisories for the overridden packages
for pkg in cross-spawn tough-cookie; do
echo "Checking advisories for $pkg..."
gh api graphql -f query='{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "'$pkg'") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
done
Length of output: 2093
48-50: LGTM! Proper commit linting setup
The added development dependencies establish a robust commit message and pre-commit checking system.
Let's verify these are the latest stable versions:
Also applies to: 58-59
✅ Verification successful
All dependencies are at their latest stable versions
The verification confirms that all the specified packages in package.json are using their latest stable versions:
- @commitlint/cli: 19.6.0 ✓
- @commitlint/config-conventional: 19.6.0 ✓
- @commitlint/types: 19.5.0 ✓
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions of added packages
for pkg in @commitlint/cli @commitlint/config-conventional @commitlint/types husky lint-staged; do
echo "Checking $pkg..."
npm view $pkg version
done
Length of output: 1031
| "scripts": { | ||
| "build": "tsup ./src/index.ts ./src/Build.ts", | ||
| "coverage": "npm test && nyc report --reporter=text-lcov | coveralls", | ||
| "postinstall": "npx install-peerdeps @wfcd/eslint-config@latest -S", |
There was a problem hiding this comment.
Reconsider postinstall script implementation
The current postinstall script has potential issues:
- Using
npxin postinstall can be problematic as it requires network access - Installing the latest version with
-Sflag during postinstall could break the dependency tree - Installing peer dependencies automatically might conflict with user's preferences
Consider these alternatives:
- Move peer dependencies to
peerDependenciesfield - Document the required peer dependencies in README
- If automation is necessary, use a separate script that users can run manually:
-"postinstall": "npx install-peerdeps @wfcd/eslint-config@latest -S",
+"setup-peers": "npx install-peerdeps @wfcd/eslint-config@latest -S",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "postinstall": "npx install-peerdeps @wfcd/eslint-config@latest -S", | |
| "setup-peers": "npx install-peerdeps @wfcd/eslint-config@latest -S", |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks).github/workflows/static.yml(2 hunks)
🔇 Additional comments (2)
.github/workflows/static.yml (2)
12-13: Verify the necessity of full history fetch
Setting fetch-depth: 0 fetches the complete repository history. While this is required for commitlint to work correctly, it increases workflow execution time and resource usage.
Consider using a shallow clone with a limited depth that covers your PR scope to optimize the workflow:
- fetch-depth: 0
+ fetch-depth: 100 # Adjust based on your typical PR size22-23: LGTM: Commitlint validation is properly configured
The commitlint step is well configured:
- Correctly uses PR's base and head SHAs for validation range
- Includes verbose flag for better debugging
- Runs after dependency installation
| - name: Validate with commitlint | ||
| run: npx commitlint --last --verbose |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider validating all new commits instead of just the last one
The current configuration with --last only validates the most recent commit. In case of multiple commits being pushed, intermediate commits might bypass validation.
Consider validating all new commits:
- run: npx commitlint --last --verbose
+ run: npx commitlint --from HEAD~$(git rev-list --count HEAD ^origin/development) --to HEAD --verbose📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Validate with commitlint | |
| run: npx commitlint --last --verbose | |
| - name: Validate with commitlint | |
| run: npx commitlint --from HEAD~$(git rev-list --count HEAD ^origin/development) --to HEAD --verbose |
What did you fix?
upgrade pre-commit frameworks
Reproduction steps
Commit some change
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
New Features
package.json.Bug Fixes
data/hash.jsonfile.data/version.jsonanddata/version.txt.Documentation
homepageandbugsURL inpackage.json.Chores