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

Allow a default message to be provided for a given pattern. #6

Merged
merged 4 commits into from
Jan 6, 2016

Conversation

jamestalmage
Copy link
Contributor

@jamestalmage
Copy link
Contributor Author

This also cleans up the way we are passing arguments around a bit by changing the signature of concreteAssert to also take the original callSpec as an argument. This reduces quite a bit of property duplication.

This attaches the wrapper function to the assertion event. This can be used with Error.captureStackTrace(), to eliminate power-assert internals from the stack trace
@twada
Copy link
Owner

twada commented Jan 4, 2016

@jamestalmage Thanks! And sorry for late response.

Is there any place to be changed in README?

@jamestalmage
Copy link
Contributor Author

Yeah, needs some docs.

I am wondering if we shouldn't also just attach their entire original specification to the event, instead of copying over individual properties. That way they can attach arbitrary data specific to their own handlers

@twada
Copy link
Owner

twada commented Jan 4, 2016

I am wondering if we shouldn't also just attach their entire original specification to the event, instead of copying over individual properties.

That's what you've done in this pull-req, right?

@@ -29,10 +30,11 @@ Decorator.prototype.enhancement = function () {
var callSpec = {
thisObj: that.receiver,
func: that.receiver[methodName],
numArgsToCapture: numberOfArgumentsToCapture(matcher),
numArgsToCapture: numberOfArgumentsToCapture(matcherSpec),
matcherSpec: matcherSpec,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've attached it here.

@twada
Copy link
Owner

twada commented Jan 5, 2016

just attach their entire original specification to the event, instead of copying over individual properties. That way they can attach arbitrary data specific to their own handlers

Now I understand and it sounds good to me ;)

@jamestalmage
Copy link
Contributor Author

OK,
I've made all the changes and attached some documentation. We will need to document it better at some point, but I'd like to get this into use in AVA and make sure we like the API before spending a lot of time on that.

@twada
Copy link
Owner

twada commented Jan 5, 2016

Thanks and agreed. If you have any changes not pushed yet, would you update this pull-req?

@jamestalmage
Copy link
Contributor Author

Sorry, I was working on a different branch and forgot.

@twada
Copy link
Owner

twada commented Jan 6, 2016

@jamestalmage looks good to me. Thank you for the great contribution!

twada added a commit that referenced this pull request Jan 6, 2016
Allow a default message to be provided for a given pattern.
@twada twada merged commit abd0e6f into twada:master Jan 6, 2016
@jamestalmage jamestalmage deleted the default-message branch January 6, 2016 03:51
@twada
Copy link
Owner

twada commented Jan 6, 2016

@jamestalmage Just released empower-core 0.3.0 🎉

I really appreciate your contributions.

@jamestalmage
Copy link
Contributor Author

Thanks @twada - I will write up an AVA PR using it in a few days. Thanks for working with us on making power-assert better for AVA.

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

Successfully merging this pull request may close these issues.

2 participants