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

Cleanup headers #340

Merged
merged 7 commits into from
Feb 24, 2021
Merged

Cleanup headers #340

merged 7 commits into from
Feb 24, 2021

Conversation

yoshuawuyts
Copy link
Member

Precursor to #335, follow-up to #315. This removes all the manual name, value, and apply methods defined on the typed headers in favor of the Header trait. This also adds conversions to easily go from &impl Header -> HeaderName / HeaderValue, which allows for more convenient use. Which also enabled us to remove a fair amount of code from the manual IntoHeaderValues implementations on various typed headers.

Example

before

let authz = BasicAuth::new(username, password);

let mut res = Response::new(200);
authz.apply(&mut res);                                         // variant 1, inverted logic
res.insert_header(authz.header_name(), authz.header_value());  // variant 2, clearer but verbose

after

let authz = BasicAuth::new(username, password);

let mut res = Response::new(200);
res.insert_header(&authz, &authz); // enabled by this PR

Future directions

This doesn't preclude changing Header::insert_header to take one Header argument rather than two separate ones. But at least under the current API it significantly simplifies usage patterns and removes lots of duplicates.

This also implements `Header for T` and provides generic conversions between `Header` and `HeaderName` / `HeaderValue`
@yoshuawuyts yoshuawuyts added the semver-major This change requires a semver major change label Feb 23, 2021
@yoshuawuyts
Copy link
Member Author

Don't think much here should be controversial -- this is mostly just a big cleanup. Going to go ahead and merge.

@yoshuawuyts yoshuawuyts merged commit e7edb1d into main Feb 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the cleanup-headers branch February 24, 2021 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major This change requires a semver major change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant