Implement _check - #2441
Conversation
Thank you for the submission, @naman-sriv! I'll review your code shortly, hang tight. |
dantharejabot
left a comment
There was a problem hiding this comment.
This is a great attempt, @naman-sriv!
I would like to request a few changes before merging your work. Please review my comments below and make the appropriate changes to your code.
After you update your code locally, follow the instructions to save your changes locally and push your changes to your fork.
When you push your changes to your fork, I'll come back for another review.
There are 12 style guide violations in your contribution. I've marked them with inline comments for your convenience.
Please revisit your code and follow the style guide best practices.
Hint: You might be able to fix some issues automatically by running npm run lint -- --fix
There was an error when running npm test. Please revisit your code and fix this error.
/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.js:1
const { exports } = require('mocha/lib/interfaces');
^
SyntaxError: Identifier 'exports' has already been declared
at Object.compileFunction (node:vm:352:18)
at wrapSafe (node:internal/modules/cjs/loader:1031:15)
at Module._compile (node:internal/modules/cjs/loader:1065:27)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.test.js:2:20)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at /var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:231:27
at Array.forEach (<anonymous>)
at Mocha.loadFiles (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:228:14)
at Mocha.run (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:514:10)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/bin/_mocha:480:18)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
at node:internal/main/run_main_module:17:47
| @@ -1,47 +1,30 @@ | |||
| const { exports } = require('mocha/lib/interfaces'); | |||
There was a problem hiding this comment.
Expected 1 empty line after require statement not followed by another require.
(learn more)
| @@ -1,47 +1,30 @@ | |||
| const { exports } = require('mocha/lib/interfaces'); | |||
There was a problem hiding this comment.
'mocha' should be listed in the project's dependencies, not devDependencies.
(learn more)
|
|
||
| exports.add = (x, y) => { | ||
| if (typeof x !== 'number') { | ||
| if(typeof x !== 'number'){ |
|
|
||
| exports.add = (x, y) => { | ||
| if (typeof x !== 'number') { | ||
| if(typeof x !== 'number'){ |
There was a problem hiding this comment.
Missing space before opening brace.
(learn more)
| exports.add = (x, y) => { | ||
| if (typeof x !== 'number') { | ||
| if(typeof x !== 'number'){ | ||
| throw new TypeError(`${x} is not a number`); |
| } | ||
| if (typeof y !== 'number') { | ||
| if(typeof y !== 'number'){ | ||
| throw new TypeError(`${y} is not a number`); |
| }; | ||
|
|
||
| exports.add = (x, y) => { | ||
| exports._check(x,y); |
| if (typeof y !== 'number') { | ||
| throw new TypeError(`${y} is not a number`); | ||
| } | ||
| exports._check(x,y); |
| if (typeof y !== 'number') { | ||
| throw new TypeError(`${y} is not a number`); | ||
| } | ||
| exports._check(x,y); |
| if (typeof y !== 'number') { | ||
| throw new TypeError(`${y} is not a number`); | ||
| } | ||
| exports._check(x,y); |
Thanks for the changes, @naman-sriv. I'm reviewing them now. |
dantharejabot
left a comment
There was a problem hiding this comment.
This is a great attempt, @naman-sriv!
I would like to request a few changes before merging your work. Please review my comments below and make the appropriate changes to your code.
After you update your code locally, follow the instructions to save your changes locally and push your changes to your fork.
When you push your changes to your fork, I'll come back for another review.
There are 1 style guide violations in your contribution. I've marked them with inline comments for your convenience.
Please revisit your code and follow the style guide best practices.
Hint: You might be able to fix some issues automatically by running npm run lint -- --fix
There was an error when running npm test. Please revisit your code and fix this error.
/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.js:1
const { exports } = require('mocha/lib/interfaces');
^
SyntaxError: Identifier 'exports' has already been declared
at Object.compileFunction (node:vm:352:18)
at wrapSafe (node:internal/modules/cjs/loader:1031:15)
at Module._compile (node:internal/modules/cjs/loader:1065:27)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.test.js:2:20)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at /var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:231:27
at Array.forEach (<anonymous>)
at Mocha.loadFiles (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:228:14)
at Mocha.run (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:514:10)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/bin/_mocha:480:18)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
at node:internal/main/run_main_module:17:47
| // Then, invoke this function inside each of the others | ||
| // HINT: you can invoke this function with exports._check() | ||
| }; | ||
| const { exports } = require('mocha/lib/interfaces'); |
There was a problem hiding this comment.
'mocha' should be listed in the project's dependencies, not devDependencies.
(learn more)
dantharejabot
left a comment
There was a problem hiding this comment.
This is a great attempt, @naman-sriv!
I would like to request a few changes before merging your work. Please review my comments below and make the appropriate changes to your code.
After you update your code locally, follow the instructions to save your changes locally and push your changes to your fork.
When you push your changes to your fork, I'll come back for another review.
There are 1 style guide violations in your contribution. I've marked them with inline comments for your convenience.
Please revisit your code and follow the style guide best practices.
Hint: You might be able to fix some issues automatically by running npm run lint -- --fix
There was an error when running npm test. Please revisit your code and fix this error.
/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.js:1
const { exports } = require('mocha/lib/interfaces');
^
SyntaxError: Identifier 'exports' has already been declared
at Object.compileFunction (node:vm:352:18)
at wrapSafe (node:internal/modules/cjs/loader:1031:15)
at Module._compile (node:internal/modules/cjs/loader:1065:27)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/src/calculator.test.js:2:20)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:94:18)
at /var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:231:27
at Array.forEach (<anonymous>)
at Mocha.loadFiles (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:228:14)
at Mocha.run (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/lib/mocha.js:514:10)
at Object.<anonymous> (/var/lib/buildkite-agent/builds/dispatchtrackimporter-1/contribute-to-open-source/contribute-to-open-source/node_modules/mocha/bin/_mocha:480:18)
at Module._compile (node:internal/modules/cjs/loader:1101:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
at node:internal/main/run_main_module:17:47
| // Then, invoke this function inside each of the others | ||
| // HINT: you can invoke this function with exports._check() | ||
| }; | ||
| const { exports } = require('mocha/lib/interfaces'); |
There was a problem hiding this comment.
'mocha' should be listed in the project's dependencies, not devDependencies.
(learn more)
Delete me and write your Pull Request message here!