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 .empty to assert interface #828

Merged
merged 4 commits into from
Oct 5, 2016
Merged

Add .empty to assert interface #828

merged 4 commits into from
Oct 5, 2016

Conversation

shvaikalesh
Copy link
Contributor

Follow-up to #812.

@meeber
Copy link
Contributor

meeber commented Oct 5, 2016

LGTM!

*
* @name isNotEmpty
* @alias notEmpty
* @param {Object|Array|String} target
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it worth it to include Map and Set here? Just to keep things explicit, what do you guys think?

@lucasfcosta
Copy link
Member

I just left a minor nitpicky comment about the accepted types of arguments for the method on the assert interface.

If you think it's not needed feel free to merge.

Despite that, LGTM! Awesome work @shvaikalesh! Thank you for this 😄

@shvaikalesh
Copy link
Contributor Author

@lucasfcosta thanks for the review, fixed that.

@lucasfcosta
Copy link
Member

LGTM.
I'll merge this since the only change since @meeber's LGTM were comments.
Thanks for you work, good job 😄

@lucasfcosta lucasfcosta merged commit 2ef786d into chaijs:master Oct 5, 2016
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.

None yet

3 participants