-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Resolve Issue #1 - Dry up the database #150
Conversation
Thank you for the submission, @DrJuneau! 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.
All of the tests are passing and the code is clean.
This is a huge improvment for our codebase's maintainability. The team is going to be stoked to see your contribution when they get back from vacation.
Well done, @DrJuneau 👏
🎉 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. This branch will be automatically deleted after one week with no merged pull requests |
Thanks for the Tutorial! |
Hello again, @DrJuneau! I want to congratulate you again for completing this challenge. Hopefully you've started to get out there and apply your knowledge to another project. Since you haven't pushed a commit in a while, I'm going to delete your branch in order to make room for others actively particiating. You'll still have access to your fork, but you won't be able to submit anymore pull requests to this branch. Don't worry though! If you decide that you do want a new branch, you can delete your fork and re-fork the original project. Happy Hacking! |
Can I work on this? |
These changes resolve issue #1 - DRY up the database.
Summary of Changes Made:
Moved duplicate error checking code into the exports._check function.
Added basic commentary to describe what the exports._check function is doing.
Changed describe.skip to describe so all the test run.
Code has been linted and tested. Runs successfully
Snapshot of test run:
$ npm test
_check
√ should throw a TypeError if arguments are not numbers
√ should be called once in "add"
√ should be called once in "subtract"
√ should be called once in "multiply"
√ should be called once in "divide"
add
√ should throw a TypeError if arguments are not numbers
√ should add two positive numbers
√ should add two negative numbers
√ should add one positive number and one negative number
subtract
√ should throw a TypeError if arguments are not numbers
√ should subtract two positive numbers
√ should subtract two negative numbers
√ should subtract one positive number and one negative number
multiply
√ should throw a TypeError if arguments are not numbers
√ should multiply two positive numbers
√ should multiply two negative numbers
√ should multiply one positive number and one negative number
divide
√ should throw a TypeError if arguments are not numbers
√ should divide two positive numbers
√ should divide two negative numbers
√ should divide one positive number and one negative number
21 passing (21ms)