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 ordered flag for members assertions #728

Merged
merged 1 commit into from
Jun 13, 2016

Conversation

meeber
Copy link
Contributor

@meeber meeber commented Jun 12, 2016

, subset
, obj
);
lengthCheck = true;
Copy link
Member

Choose a reason for hiding this comment

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

Nice work using lengthCheck at this assertion. The code looks more clean 👍

@lucasfcosta
Copy link
Member

lucasfcosta commented Jun 12, 2016

Awesome work, as always. I went through it all and I couldn't even find a single typo.
Nice work reducing the members assertion to a single call to assert at the end, btw.
LGTM.

return subset.every(function(elem) {
function isSubsetOf(subset, superset, cmp, ordered) {
return subset.every(function(elem, idx) {
if (ordered) return cmp ? cmp(elem, superset[idx]) : elem === superset[idx];
Copy link
Member

Choose a reason for hiding this comment

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

I'll leave this here just to make other reviewer's job easier.
This line is related to the behaviour we talked about at this post.

@meeber
Copy link
Contributor Author

meeber commented Jun 12, 2016

@lucasfcosta Thank you for reviewing :D

@keithamus
Copy link
Member

Great work @meeber 😄. LGTM!

@keithamus keithamus merged commit c273846 into chaijs:master Jun 13, 2016
@meeber meeber deleted the ordered-members branch August 6, 2017 13:47
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.

3 participants