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

Replace default_headers_mut with default_headers #186

Merged
merged 4 commits into from
May 10, 2020

Conversation

sagebind
Copy link
Owner

@sagebind sagebind commented May 7, 2020

Thinking about future compatibility, we might want to leverage interceptors someday as an implementation detail for how default headers are set. To avoid locking us in the current implementation, do not expose the default HeaderMap as a direct reference. Instead, for users who want to set headers in bulk, provide default_headers which takes an iterator of key-value pairs and replaces all existing headers. That allows users to do the same kind of things without relying on a mutable HeaderMap being exposed.

Also tweak some docs.

Thinking about future compatibility, we might want to leverage interceptors as an implementation detail for how default headers are set. To avoid locking us in the current implementation, do not expose the default `HeaderMap` as a direct reference. Instead, for users who want to set headers in bulk, provide `default_headers` which takes an iterator of key-value pairs and replaces all existing headers. That allows users to do the same kind of things without relying on a mutable `HeaderMap` being exposed.
@codecov
Copy link

codecov bot commented May 7, 2020

Codecov Report

Merging #186 into master will increase coverage by 1.68%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
+ Coverage   67.97%   69.65%   +1.68%     
==========================================
  Files          33       33              
  Lines        1808     1849      +41     
==========================================
+ Hits         1229     1288      +59     
+ Misses        579      561      -18     
Impacted Files Coverage Δ
src/error.rs 24.70% <0.00%> (+0.56%) ⬆️
src/client.rs 61.80% <28.57%> (-3.25%) ⬇️
src/cookies/mod.rs 82.79% <0.00%> (-0.45%) ⬇️
src/text.rs 70.68% <0.00%> (ø)
src/config/dns.rs 0.00% <0.00%> (ø)
src/auth.rs 61.90% <0.00%> (+0.96%) ⬆️
src/handler.rs 73.62% <0.00%> (+1.19%) ⬆️
src/agent.rs 72.22% <0.00%> (+1.44%) ⬆️
src/metrics.rs 33.33% <0.00%> (+8.94%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9254dcf...07541db. Read the comment docs.

@sagebind sagebind merged commit 5e2c0c0 into master May 10, 2020
@sagebind sagebind deleted the default-headers-builder branch May 10, 2020 01:40
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.

1 participant