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

docs: fix usage page to match latest version's default adapter specification #1447

Merged

Conversation

mi-wada
Copy link
Contributor

@mi-wada mi-wada commented Aug 25, 2022

Description

Since #1366, the default value for default_adapter is :net_http.

Additional Notes

The added Adapters section was referenced here.

@olleolleolle
Copy link
Member

I like this, thank you!

As a follow-up, when looking at the Adapters page, I think as a reader, I would like a snippet of code in context, to take the mystery out of it. Setting a different default_adapter and so on.

https://lostisland.github.io/faraday/adapters/

@iMacTia Do you agree that something more concrete on that page could guide a half-knowledgeable reader to the water?

@mi-wada mi-wada force-pushed the mi-wada/fix-doc-to-match-latest-version branch from 22978f0 to 07f78ac Compare August 30, 2022 03:56
@mi-wada
Copy link
Contributor Author

mi-wada commented Aug 30, 2022

@olleolleolle

As a follow-up, when looking at the Adapters page, I think as a reader, I would like a snippet of code in context, to take the mystery out of it. Setting a different default_adapter and so on.

https://lostisland.github.io/faraday/adapters/

Sure looks better that way!

07f78ac

I have added a doc about this and would appreciate a review.

@iMacTia
Copy link
Member

iMacTia commented Sep 5, 2022

This looks great! Thank you for improving the doc @mi-wada and thanks @olleolleolle for the suggestion 🙏 !

@iMacTia iMacTia merged commit 7926e3a into lostisland:main Sep 5, 2022
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