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

Fix deepEqual on empty objects. Fixes #127. #128

Merged
merged 2 commits into from Feb 26, 2015

Conversation

@Marsup
Copy link
Member

Marsup commented Feb 22, 2015

No description provided.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Feb 22, 2015

LGTM

EDIT: Object.keys() only returns enumerable properties. It might be worth looking into Object.getOwnPropertyNames(), which returns all properties.

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Feb 22, 2015

Sure, makes sense, I can change that.

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Feb 22, 2015

This might be a breaking change for some people. Look at the test I had to change and tell me if we want to keep it.

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Feb 22, 2015

We also probably want to do that in clone btw.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented Feb 23, 2015

It might be breaking for some people but it is expected behaviour (?) so in that sense it is more of a patch

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Feb 23, 2015

My vote is for a patch. It will only break existing code if that code depends on incorrect behavior.

@Marsup

This comment has been minimized.

Copy link
Member Author

Marsup commented Feb 26, 2015

@nlf This one can be merged then, I think the clone should be in another PR.

@nlf nlf added the bug label Feb 26, 2015
@nlf nlf added this to the 2.11.1 milestone Feb 26, 2015
@nlf nlf self-assigned this Feb 26, 2015
nlf added a commit that referenced this pull request Feb 26, 2015
Fix deepEqual on empty objects. Fixes #127.
@nlf nlf merged commit 0f341f7 into hapijs:master Feb 26, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Marsup Marsup deleted the Marsup:empty-object-deepEqual branch Feb 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.