-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIX : #1 Implement _check #2176
Conversation
Thank you for the submission, @eugene4545! I'll review your code shortly, hang tight. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great attempt, @eugene4545!
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 4 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
All the tests are passing. Nice job!
src/calculator.js
Outdated
if (typeof x !== 'number') { | ||
throw new TypeError(`${x} is not a number`); | ||
} | ||
if (typeof y !== 'number') { | ||
throw new TypeError(`${y} is not a number`); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing spaces not allowed.
(learn more)
src/calculator.js
Outdated
if (typeof x !== 'number') { | ||
throw new TypeError(`${x} is not a number`); | ||
} | ||
if (typeof y !== 'number') { | ||
throw new TypeError(`${y} is not a number`); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 2 spaces but found 0.
(learn more)
src/calculator.js
Outdated
if (typeof x !== 'number') { | ||
throw new TypeError(`${x} is not a number`); | ||
} | ||
if (typeof y !== 'number') { | ||
throw new TypeError(`${y} is not a number`); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Block must not be padded by blank lines.
(learn more)
src/calculator.js
Outdated
return x / y; | ||
}; | ||
|
||
module.exports = exports; | ||
module.exports = exports; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline required at end of file but not found.
(learn more)
Thanks for the changes, @eugene4545. I'm reviewing them now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Thanks for your contribution @eugene4545!
Your code style completely follows the style guide. Nice job!
All the tests are passing. Nice job!
🎉 You did it! 🎉You're an open source contributor now! Whether this was your first pull request, or you’re just looking for new ways to contribute, I hope you’re inspired to take action. Don't forget to say thanks when a maintainer puts effort into helping you, even if a contribution doesn't get accepted. Open source is made by people like you: one issue, pull request, comment, and +1 at a time. What's next?Find your next project:
Learn from other great community members:
Elevate your Git game:
Questions? Comments? Concerns?I'm always open to feedback. If you had a good time with the exercise, or found some room for improvement, please email me. Want to start over? Just delete your fork. |
Delete me and write your Pull Request message here!