Repository navigation
feat: add linter flow (#61) - #62
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Caution Review failedThe pull request is closed. WalkthroughAdds ESLint configuration, npm scripts, and a GitHub Actions workflow for linting; standardizes formatting across files and removes unused Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/lint.yml:
- Line 19: Update the GitHub Actions workflow matrix entry that sets
node-version (the node-version: '18' value in the lint workflow) to a supported
LTS like '22' or '24' (e.g., change node-version: '18' → node-version: '22') so
the CI runs on a current LTS; ensure any job matrices or tooling that depend on
Node.js version are adjusted accordingly.
In @api/routes/products.js:
- Around line 44-48: The response JSON in the router.patch('/:productID' route
contains a message string with an unintended leading space; update the message
value in the route handler (router.patch('/:productID', ...)) from ' Product
updated successfully!' to 'Product updated successfully!' so the API returns the
correct string without the extra space.
- Around line 52-56: The response message in the DELETE route handler for
router.delete('/:productID', (req, res) => { ... }) contains a leading space ("
Product deleted successfully."); remove the extra leading space from the message
string so it reads "Product deleted successfully." to ensure consistent
formatting in JSON responses.
In @app.js:
- Around line 25-33: The error-handling middleware passed to app.use is declared
with only three parameters so Express won't recognize it; update the handler
signature to include the fourth parameter `next` (e.g., change the callback for
app.use to (err, req, res, next) => { ... }) and, to avoid ESLint
no-unused-vars, either prefix `next` with an underscore or add an eslint disable
comment for that line so the required but unused parameter is not flagged.
🧹 Nitpick comments (3)
package.json (1)
22-26: Consider moving test libraries to devDependencies.
chaiandmochaare testing libraries and should be indevDependenciesrather thandependenciesto avoid shipping them in production builds.Suggested change
"dependencies": { "body-parser": "^2.2.2", - "chai": "^6.2.2", "express": "^5.2.1", - "mocha": "^11.7.5", "mongoose": "^9.1.2", "morgan": "^1.10.1" }, "devDependencies": { + "chai": "^6.2.2", "eslint": "^9.39.2" + "mocha": "^11.7.5", }eslint.config.js (1)
9-22: Consider using theglobalspackage for cleaner Node.js globals definition.Instead of manually listing Node.js globals, you could use the
globalspackage which provides predefined sets.Optional improvement
npm install --save-dev globalsconst js = require('@eslint/js'); +const globals = require('globals'); module.exports = [ js.configs.recommended, { languageOptions: { ecmaVersion: 'latest', sourceType: 'commonjs', - globals: { - console: 'readonly', - process: 'readonly', - require: 'readonly', - module: 'readonly', - __dirname: 'readonly', - __filename: 'readonly', - exports: 'readonly', - Buffer: 'readonly', - setTimeout: 'readonly', - setInterval: 'readonly', - clearTimeout: 'readonly', - clearInterval: 'readonly' - } + globals: { + ...globals.node + } },api/routes/products.js (1)
26-26: Use strict equality (===) instead of loose equality (==).
req.params.productIDis always a string, so comparing with== 123relies on type coercion. Using=== '123'(or=== 123after parsing) is safer and more explicit.Suggested fix
- if (id == 123) { + if (id === '123') {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/lint.ymlREADME.mdapi/routes/orders.jsapi/routes/products.jsapi/routes/visitors.jsapp.jseslint.config.jspackage.json
🧰 Additional context used
🧬 Code graph analysis (2)
api/routes/products.js (2)
api/routes/orders.js (1)
router(2-2)api/routes/visitors.js (1)
router(2-2)
api/routes/orders.js (2)
api/routes/products.js (1)
router(2-2)api/routes/visitors.js (1)
router(2-2)
🪛 GitHub Check: Codacy Static Code Analysis
eslint.config.js
[warning] 1-1: eslint.config.js#L1
Require statement not part of import statement.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (15)
api/routes/visitors.js (2)
7-25: LGTM!The formatting changes are consistent with the new ESLint configuration. The route logic remains unchanged.
28-37: LGTM!The
/statsendpoint formatting is now consistent with the project's linting rules.package.json (2)
9-10: LGTM!The lint scripts are correctly configured for running ESLint checks and auto-fixing issues.
28-30: No action needed — ESLint 9.39.2 is the current latest version.The version
^9.39.2is valid and up-to-date.Likely an incorrect or invalid review comment.
app.js (2)
1-14: LGTM!The imports and route mounting are correctly formatted.
17-24: LGTM!The 404 handler correctly creates an error and passes it to the next middleware via
next(err).eslint.config.js (1)
1-35: LGTM!The ESLint flat config is well-structured with sensible defaults for a Node.js CommonJS project. The rules align with the formatting changes across the codebase.
api/routes/orders.js (4)
4-11: LGTM!Removing the unused
nextparameter is correct for this simple route handler that always responds directly.
13-20: LGTM!The POST handler is correctly simplified.
23-30: LGTM!The GET by ID handler is correctly formatted.
32-39: LGTM!The DELETE handler is correctly formatted.
api/routes/products.js (2)
4-11: LGTM!The route handler signature change removing the unused
nextparameter is appropriate since it's not used in the handler logic.
13-19: LGTM!Clean POST handler with proper 201 status code for resource creation.
README.md (1)
3-3: LGTM!The lint badge is correctly formatted and consistent with the existing Dependabot and Version badges. It properly links to the new lint workflow.
.github/workflows/lint.yml (1)
1-26: Well-structured lint workflow.The workflow configuration follows best practices: using
npm cifor deterministic installs, enabling npm caching, and using current action versions.
| router.delete('/:productID', (req, res) => { | ||
| res.status(200).json({ | ||
|
|
||
| message: " Product deleted successfully." | ||
| }) | ||
| message: ' Product deleted successfully.' | ||
| }); |
There was a problem hiding this comment.
Remove leading space in message string.
Same issue here — ' Product deleted successfully.' has a leading space.
Suggested fix
- message: ' Product deleted successfully.'
+ message: 'Product deleted successfully.'🤖 Prompt for AI Agents
In @api/routes/products.js around lines 52 - 56, The response message in the
DELETE route handler for router.delete('/:productID', (req, res) => { ... })
contains a leading space (" Product deleted successfully."); remove the extra
leading space from the message string so it reads "Product deleted
successfully." to ensure consistent formatting in JSON responses.
Summary by CodeRabbit
New Features
Documentation
Chores
lintandlint:fixnpm scripts.✏️ Tip: You can customize this high-level summary in your review settings.