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

Jump to bottom

update middleware docs #1277

Merged
merged 3 commits into from May 2, 2021
Merged

update middleware docs #1277

merged 3 commits into from May 2, 2021

Conversation

gurgeous
Copy link
Contributor

@gurgeous gurgeous commented May 1, 2021

Per the earlier discussion:

  1. Copied some of the language from Usage
  2. Show off more middleware and features
  3. Linked to awesome-faraday (PR pending over there too)
  4. Digression on request, response and adapter namespaces

Thanks guys!

olleolleolle
olleolleolle reviewed May 1, 2021
View reviewed changes
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start of getting the documentation to a place which would have helped "old you" of two weeks ago! Thanks for that.

docs/middleware/index.md Show resolved Hide resolved
docs/middleware/index.md
often provided by external gems, like the
[faraday-middleware](https://github.com/lostisland/faraday_middleware) gem.

We also have [great documentation](list) for the middleware that ships with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: does this link work? As a casual reader on my phone, I can't recall if we have a "list" link or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but thank you for asking. I appreciate the attention to detail! I am using jekyll serve --livereload to test. It links to the Available Middleware page.

olleolleolle
olleolleolle approved these changes May 2, 2021
View reviewed changes
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR improves the reader's chances of grasping what Connection is in the context of Faraday. Thanks!

@olleolleolle
Copy link
Member

@gurgeous Thanks! I'll merge this change!

@olleolleolle olleolleolle merged commit 11d793d into lostisland:main May 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olleolleolle olleolleolle olleolleolle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gurgeous @olleolleolle