Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Value == bool #138

Merged
merged 1 commit into from Oct 4, 2019
Merged

Value == bool #138

merged 1 commit into from Oct 4, 2019

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Oct 3, 2019 •
edited

Number and String values can be compared, so why not Bool?

@CAD97
Copy link
Contributor Author

CAD97 commented Oct 3, 2019

The two commits implement different approaches for implementing it; I think reusing partialeq_numeric! (second commit) is the better way to go.

dtolnay
dtolnay reviewed Oct 4, 2019
View reviewed changes
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

Could you update this to remove impl PartialEq<Value> for bool? The existing PartialEq<Value> impls we have are a bad idea and I don't want to add more. #140

@CAD97
Copy link
Contributor Author

CAD97 commented Oct 4, 2019

This now only adds the impl PartialEq<bool> for Value.

dtolnay
dtolnay approved these changes Oct 4, 2019
View reviewed changes
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit 5f25a69 into dtolnay:master Oct 4, 2019
@CAD97 CAD97 deleted the patch-1 branch October 4, 2019 20:26
@dtolnay
Copy link
Owner

dtolnay commented Oct 4, 2019

Published in serde_yaml 0.8.10.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dtolnay dtolnay dtolnay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@CAD97 @dtolnay