Skip to content
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

Add tests using ava #2

Merged
merged 7 commits into from
Nov 20, 2018
Merged

Conversation

bendrucker
Copy link
Contributor

@bendrucker bendrucker commented Nov 17, 2018

Hi there, maintainer of hapi-raven here! Someone asked about support for @sentry/node and I happened upon your package. I'd like to deprecate hapi-raven and direct users over here. I'd like to see this project reach 1.0 before I do that.

I've added tests here. I chose ava since I noticed you'd used it before on a project. They have some major changes coming up in a 1.0 so I figured the prerelease made the most sense.

I tried to focus on the major branches/features. Happy to come back and add more tests for the various options in a later PR.

@fiws fiws added the enhancement New feature or request label Nov 20, 2018
Copy link
Contributor

@guischdi guischdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bendrucker
sorry for the late response. Thanks a lot your PR!
Here is some (mainly syntactical) issues. Looking forward to your changes!

package.json Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Show resolved Hide resolved
@guischdi guischdi self-assigned this Nov 20, 2018
@guischdi
Copy link
Contributor

I recognized that you do not use the method shorthand on object initialization (handler() { ... instead of handler: () => { ... as mentioned before). Would you please correct that?

Also, would you mind using eslint-plugin-ava? Would have helped with some tests linting.

@bendrucker
Copy link
Contributor Author

Updated!

@guischdi guischdi merged commit 61148cb into hydra-newmedia:master Nov 20, 2018
@bendrucker bendrucker deleted the add-tests branch November 20, 2018 17:42
@guischdi
Copy link
Contributor

Just released a new version 0.1.2!

@bendrucker bendrucker mentioned this pull request Nov 20, 2018
@guischdi
Copy link
Contributor

Just released a new version 1.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants