Skip to content

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented Jan 22, 2025

The proxy::http::Version type is very similar to the HTTP crate's Version type, though it is more constrained so that the proxy may exhaustively match on it. This change renames the module to http::variant to avoid confusion with the HTTP crate's Version type.

To disambiguate the HTTP version type, the proxy::http::Version type is renamed to proxy::http::Variant.

The proxy::http::Version type is very similar to the HTTP crate's Version type,
though it is more constrained so that the proxy may exhaustively match on it.
This change renames the module to http::variant to avoid confusion with the HTTP
crate's Version type.

To disambiguate the HTTP version type, the proxy::http::Version type is renamed
to proxy::http::Variant.
@olix0r olix0r requested a review from a team as a code owner January 22, 2025 15:31
@olix0r olix0r requested review from cratelyn and removed request for a team January 22, 2025 15:31
struct HttpSidecar {
orig_dst: OrigDstAddr,
version: http::Version,
version: http::Variant,
Copy link
Member

@cratelyn cratelyn Jan 22, 2025

Choose a reason for hiding this comment

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

🍰 TIOLI: should this field, and others like it, also be renamed to variant? i'm ambivalent about this suggestion, personally, but if the type is being renamed perhaps fields like this should follow suit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't really think there's tremendous value in the type and field names matching. The intent is still that it's an HTTP version, and the type disambiguates what that means, exactly.

@olix0r olix0r merged commit d436b93 into main Jan 22, 2025
17 checks passed
@olix0r olix0r deleted the ver/http-variant branch January 22, 2025 15:55
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