Conversation
There was a problem hiding this comment.
The PR removes husky and eslint from devDependencies while the scripts section still references both ("prepare": "husky" and "lint": "npx eslint ."), causing broken builds in clean environments. Additionally, pinning @cap-js/cds-types to "*" is overly permissive and risks unexpected breakage from future major versions.
PR Bot Information
Version: 1.27.2
- File Content Strategy: Full file content
- Event Trigger:
pull_request.opened - Correlation ID:
8d2613c0-7add-11f1-9c0a-9e2e0ccb4e36 - LLM:
anthropic--claude-4.6-sonnet
| "@cap-js/cds-test": "^1", | ||
| "@cap-js/cds-types": "*", | ||
| "@cap-js/hana": ">=2.7", | ||
| "@cap-js/sqlite": ">=2", | ||
| "@eslint/js": "^10" |
There was a problem hiding this comment.
Bug: husky removed from devDependencies but still referenced in scripts.prepare
The "prepare": "husky" script at line 22 will fail for anyone running npm install because husky is no longer listed as a dependency. Either re-add husky to devDependencies or remove/update the prepare script accordingly.
| "@cap-js/cds-test": "^1", | |
| "@cap-js/cds-types": "*", | |
| "@cap-js/hana": ">=2.7", | |
| "@cap-js/sqlite": ">=2", | |
| "@eslint/js": "^10" | |
| "@cap-js/cds-test": "^1", | |
| "@cap-js/cds-types": "*", | |
| "@cap-js/hana": ">=2.7", | |
| "@cap-js/sqlite": ">=2", | |
| "@eslint/js": "^10", | |
| "husky": "^9.1.7" |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| "express": "^4.18.2", | ||
| "husky": "^9.1.7" | ||
| "@cap-js/cds-test": "^1", | ||
| "@cap-js/cds-types": "*", |
There was a problem hiding this comment.
Best Practices: @cap-js/cds-types pinned to "*" installs any version including potentially breaking major releases.
Using "*" means any version, including future major versions with breaking changes, will be installed. This is riskier than the previous ^0.17.0 range. Consider using a more constrained range like ">=0.17" or "^0" to stay on compatible versions.
| "@cap-js/cds-types": "*", | |
| "@cap-js/cds-types": ">=0.17", |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| "@cap-js/cds-test": "^1", | ||
| "@cap-js/cds-types": "*", | ||
| "@cap-js/hana": ">=2.7", | ||
| "@cap-js/sqlite": ">=2", | ||
| "@eslint/js": "^10" |
There was a problem hiding this comment.
Bug: eslint removed from devDependencies but the lint script invokes it.
The "lint": "npx eslint ." script (line 18) relies on eslint being available. Removing it means the linter may not be installed during npm install, causing npm run lint to fail in a clean environment. Re-add eslint to devDependencies.
| "@cap-js/cds-test": "^1", | |
| "@cap-js/cds-types": "*", | |
| "@cap-js/hana": ">=2.7", | |
| "@cap-js/sqlite": ">=2", | |
| "@eslint/js": "^10" | |
| "@cap-js/cds-test": "^1", | |
| "@cap-js/cds-types": ">=0.17", | |
| "@cap-js/hana": ">=2.7", | |
| "@cap-js/sqlite": ">=2", | |
| "@eslint/js": "^10", | |
| "eslint": "^10", | |
| "husky": "^9.1.7" |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
This reverts commit 6b97bd9.
| "express": "^4.18.2", | ||
| "husky": "^9.1.7" | ||
| "@cap-js/cds-test": "^1", | ||
| "@cap-js/cds-types": "*", |
There was a problem hiding this comment.
| "@cap-js/cds-types": "*", |
As a JavaScript project we do/should not need @sap/cds-types at all.
These are rather for typescript projects.
|
If there are different tastes re husky and prettier we should come to a conclusion as a team |
eslintthroughnpx→ removed dev dependency@cap-js/sqliteand/hanaexpressdev dependency@sap/cds-typesin JavaScript projects → see belowhusky→ removed the dev dependency → see comment