Skip to content

Conversation

@Marsup
Copy link
Contributor

@Marsup 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.

internals.addMethod(word, method);
});

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@cjihrig
Copy link
Contributor

cjihrig commented Apr 5, 2016

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

@Marsup
Copy link
Contributor 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 New functionality or improvement 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
@cjihrig cjihrig modified the milestone: 2.2.0 Apr 6, 2016
@AdriVanHoudt
Copy link
Contributor

no api docs update 😢

@cjihrig
Copy link
Contributor

cjihrig commented Apr 6, 2016

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

@Marsup
Copy link
Contributor Author

Marsup commented Apr 6, 2016

Dammit ! I always forget that ! Sorry.

@Marsup Marsup deleted the error-type branch April 6, 2016 13:38
@Marsup
Copy link
Contributor Author

Marsup commented Apr 6, 2016

Done as #64.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 6, 2016

@Marsup just published [email protected]

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants