-
Notifications
You must be signed in to change notification settings - Fork 52
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
Better error handling #21
base: master
Are you sure you want to change the base?
Conversation
Allows testing for Promise rejections cleanly
This makes failure cases more clear to the calling code
Check for thrown Error()s instead of nulls.
Pull Request Test Coverage Report for Build 50
💛 - Coveralls |
That's brilliant! Thank you! It looks like you may have edited the build files instead of the pre-babelised source code in the How would you like to proceed? |
I've updated both (source then babelize), it's just that I didn't notice a gitignore for the build directory so I thought you kept both updated together 😅 The only commit that affects the build files would be the last one so the rest can be used instead :D |
Ahh sorry, Understood. |
Did you write a script to help you refactor the code? I'm curious about your approach to the problem. |
To be honest I was changing lines manually 🤣 |
Nice :) |
I'll review this carefully and try to make some additions as well, prior to merging. Perhaps we should notify the users about an upcoming API surface change, too. |
Yes that's a good idea. I'll keep building with the API and add more endpoints if necessary so all of it can be made into one release :) |
Sounds good. I'm really glad to have a collaborator on the project. |
Very cool. |
For issue #5
Functions now throw Error objects instead of returning nulls
Potential issues:
This PR isn't intended to be merged right away until reviewed for more issues