fix: configure patches for linter#1027
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your submission because the splitInteger functionality and its Jest tests meet the core requirements and run as expected. Nice job covering the main behaviors (equal splits, non-equal ascending splits, and cases where value < parts).
Minor improvements (non-blocking):
- eslint.config.js: If your project isn’t ESM, switch to CommonJS (
require/module.exports), and mapglobalsvalues to strings like'readonly'to avoid ESLint flat-config interpretation issues. - .eslintrc.js: Consider using
'off'instead of0for rule levels for clarity. - .github/workflows/test.yml: Consider adding Node 18.x to the matrix and using newer action versions plus
npm cifor CI reliability.
Optional enhancement: Add a property-based test that asserts sum equals value, length equals numberOfParts, sorted order, and max - min <= 1 across more inputs. Overall, well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @@ -0,0 +1,22 @@ | |||
| import globals from "globals"; | |||
There was a problem hiding this comment.
You're using top-level ES module syntax (import / export default) here. Ensure your project is configured for ESM (package.json contains "type": "module") or convert this file to CommonJS (const globals = require('globals') / module.exports = [...]). If your environment doesn't support ESM, ESLint will fail to load this config. Also, note that the globals package is CommonJS — when using ESM interop you may want to import it as import * as globals from 'globals' to avoid getting undefined (depending on your bundler/Node version).
| globals: { | ||
| ...globals.node, | ||
| ...globals.jest, |
There was a problem hiding this comment.
languageOptions.globals in the flat config usually expects values like "readonly" or "writable" (strings). The globals package exports objects whose values are booleans (or may not match the string form). Spreading ...globals.node and ...globals.jest here will put those boolean values into the config which may not be interpreted as intended by ESLint. Consider mapping them, for example:
const mapToReadonly = (g) => Object.fromEntries(Object.keys(g).map(k => [k, 'readonly']));
...
globals: {
...mapToReadonly(globals.node),
...mapToReadonly(globals.jest),
},Or verify your ESLint version accepts boolean globals in the flat config format.
| strategy: | ||
| matrix: | ||
| node-version: [12.x] | ||
| node-version: [20.x] |
There was a problem hiding this comment.
The workflow matrix only includes Node 20.x. Consider expanding this to include at least the current LTS (for example, 18.x) so your tests run against multiple supported Node versions and surface compatibility issues earlier. If you intentionally want to test only Node 20, add a short comment explaining that decision.
Description