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

chore(http): rename http/version to http/variant #3555

Merged
merged 1 commit into from
Jan 22, 2025
Merged

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
@@ -28,7 +28,7 @@ struct Sidecar {
#[derive(Clone, Debug)]
struct HttpSidecar {
orig_dst: OrigDstAddr,
version: http::Version,
version: http::Variant,
Copy link
Collaborator

@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.

2 participants