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 error type (fixes #61) #62

Merged
merged 2 commits into from Apr 6, 2016

Conversation

@Marsup
Copy link
Member

Marsup commented Apr 5, 2016

Mostly ripoff of the throw code with small adjustments in the tests because I considered not passing a type should still check whether that's an error type or not, I'd argue the throw logic is wrong on this side.

@@ -169,6 +169,21 @@ internals.addMethod = function (names, fn) {
internals.addMethod(word, method);
});

internals.addMethod('error', function (/*type, message*/) {

This comment has been minimized.

Copy link
@cjihrig

cjihrig Apr 5, 2016

Contributor

I feel like there is some DRY that could be applied here.

This comment has been minimized.

Copy link
@Marsup

Marsup Apr 6, 2016

Author Member

Considering the logic and the error strings are slightly different, I can't see any re-use.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 5, 2016

Can you add a test showing expect(foo).to.not.be.an.error()

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Apr 6, 2016

It should be OK now, the logic on the not was wrong, so good gatekeeping here ;)

@cjihrig cjihrig self-assigned this Apr 6, 2016
@cjihrig cjihrig added the feature label Apr 6, 2016
@cjihrig cjihrig added this to the 2.2.0 milestone Apr 6, 2016
@cjihrig cjihrig merged commit 35c0ac7 into hapijs:master Apr 6, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cjihrig cjihrig modified the milestone: 2.2.0 Apr 6, 2016
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented Apr 6, 2016

no api docs update 😢

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 6, 2016

Good catch. Created #63 and assigned to @Marsup to track the documentation update.

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Apr 6, 2016

Dammit ! I always forget that ! Sorry.

@Marsup Marsup deleted the Marsup:error-type branch Apr 6, 2016
@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Apr 6, 2016

Done as #64.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Apr 6, 2016

@Marsup just published code@2.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.