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

Req.Request, Req.Response: Change headers to be maps #224

Merged
merged 6 commits into from
Aug 25, 2023
Merged

Conversation

wojtekmach
Copy link
Owner

Closes #223

@wojtekmach wojtekmach merged commit 8e87950 into main Aug 25, 2023
2 checks passed
@wojtekmach wojtekmach deleted the wm-headers branch August 25, 2023 19:44
@florish
Copy link
Contributor

florish commented Sep 4, 2023

Hi @wojtekmach, thanks for all the work put into Req!

We're in the processing of migrating from 0.3 to 0.4, and in the process I assumed that the :headers option in Req.new/1 and Req.update/2 would also have been changed to expect map data instead of lists. E.g.:

req = Req.new(url: "https://example.com", headers: %{"accept" => ["application/json"]})
Req.update(req, headers: %{"foo" => ["bar"]})

Is there a specific reason that these values are still lists in 0.4.x?

@wojtekmach
Copy link
Owner Author

Both are supported! (If maps are not, it’s a bug.) the idea is passing eg accept: “application/json” is much more concise than the equivalent map with, notably, list of values.

@wojtekmach
Copy link
Owner Author

In hindsight this is not necessarily super obvious so a PR that updates docs is welcome as always!

@florish
Copy link
Contributor

florish commented Sep 4, 2023

Ah, thanks! Would you like me to replace the existing list examples by maps or would you prefer to have examples added using maps?

florish added a commit to florish/req that referenced this pull request Sep 4, 2023
@florish
Copy link
Contributor

florish commented Sep 4, 2023

@wojtekmach See #241 for PR, feel free to change anything you like!

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.

Change headers to maps
2 participants