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 PartialEq between &StatusCode and u16 #319

Closed
wants to merge 1 commit into from

Conversation

jbr
Copy link
Member

@jbr jbr commented Jan 16, 2021

No description provided.

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Jan 16, 2021

StatusCode is a thin wrapper around u16, and implements Clone. Simply dereferencing one side, or borrowing another should already work in all cases right?

assert_eq!(*status, 400);
assert_eq!(status, &400);

@joshtriplett
Copy link
Member

I think we should maintain the distinction between these two types, since it's not especially difficult to dereference StatusCode when comparing it. Is there a use case where that makes code substantially more onerous?

@jbr
Copy link
Member Author

jbr commented Jan 23, 2021

It's been a bit since I needed this, but I believe it was either for matching or for equality testing between nested types like Option<&StatusCode> without having to .as_deref(). Not critically important, but more convenient

@yoshuawuyts
Copy link
Member

Oh, if we have an API anywhere here which returns Option<&u16> we should def change it to be Option<u16> instead. @jbr did you recall whether this was in an http-rs library?

@jbr
Copy link
Member Author

jbr commented Jan 24, 2021

It wasn't in http-rs. The function was returning Option<&StatusCode> and it was convenient to be able to test equality Some(200) or whatever. The other PartialEq was just for parity. Not important behavior, I just didn't see any downside

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