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

External get func name #12

Merged
merged 2 commits into from
Oct 16, 2016
Merged

External get func name #12

merged 2 commits into from
Oct 16, 2016

Conversation

lucasfcosta
Copy link
Member

This includes a small refactor for this module to use the external get-func-name module.
Please notice that this has been made on top of #11.

@meeber
Copy link

meeber commented Oct 16, 2016

Looks like these need to be rebased?

constructorName = getFunctionName(errorLike);
if (constructorName === '') {
var newConstructorName = getFunctionName(new errorLike()); // eslint-disable-line new-cap
constructorName = newConstructorName ? newConstructorName : constructorName;
Copy link
Member

Choose a reason for hiding this comment

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

LGTM. My minor nitpick is this that could be constructorName = newConstructorName || constructorName; which I think would read better.

@lucasfcosta
Copy link
Member Author

@keithamus Yes! That looks better indeed! Thanks for the help.

I have also rebased both PRs on master, they should be ready for a merge now.

@vieiralucas
Copy link
Member

LGTM!

@meeber
Copy link

meeber commented Oct 16, 2016

LGTM

@meeber
Copy link

meeber commented Oct 16, 2016

@keithamus It says I don't have write access and therefore can't merge. Halp!

@vieiralucas
Copy link
Member

Me to 🐼

@lucasfcosta
Copy link
Member Author

@meeber @vieiralucas +1

@keithamus
Copy link
Member

Sorry, forgot to add the team to this repo. You should all be able to merge now 😄

@keithamus keithamus merged commit c86b2dc into chaijs:master Oct 16, 2016
@lucasfcosta
Copy link
Member Author

Can anyone restart the build?
I'm trying to reproduce the error on my machine but I can't.
I'm running IE11 on Win7 but it won't fail, maybe saucelabs is bugged, idk.

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.

4 participants