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

.members should show diff #702

Merged
merged 1 commit into from May 10, 2016
Merged

.members should show diff #702

merged 1 commit into from May 10, 2016

Conversation

pgarrison
Copy link

It would be great to show what's missing; this works for me. Comments are welcome.
I'm not sure why the previous code had #{act} instead of #{exp} or why it apparently swapped expected/actual.

@meeber
Copy link
Contributor

meeber commented May 10, 2016

@pgarrison Thanks for the PR!

The issue with showDiff not being true here is indeed an issue but has been fixed in the upcoming 4.x.x release per #574.

The issue with the transposed actual and expected arguments for the members assertion is also a known issue with a PR submitted to fix it per #511. However, the submitter of that PR hasn't responded in a month so I'd be happy to close that one and merge this one instead.

Would you be willing to update this PR to remove the true showDiff changes, since that conflicts with the existing fix in #574?

Thanks!

@pgarrison
Copy link
Author

pgarrison commented May 10, 2016

Ah okay, didn't see the 4.x.x branch--should this be there instead?

@meeber
Copy link
Contributor

meeber commented May 10, 2016

Nahhh, fixing the swapped actual and expected bug can go onto master.

Thank you for asking though!

@pgarrison
Copy link
Author

Alright. It no longer sets showDiff.

@meeber
Copy link
Contributor

meeber commented May 10, 2016

@pgarrison Thank you, sir. LGTM.

@keithamus
Copy link
Member

LGTM!

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