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(FAQ): add a FAQ entry based on @vytas7 discussion answer #1982

Merged
merged 4 commits into from
Nov 7, 2021

Conversation

CaselIT
Copy link
Member

@CaselIT CaselIT commented Oct 27, 2021

#1981 (comment)

Summary of Changes

Add a faq item based on @vytas7 answer at #1981 (comment)

@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #1982 (43046c4) into master (02942f3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1982   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        63           
  Lines         6669      6669           
  Branches      1079      1079           
=========================================
  Hits          6669      6669           

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 02942f3...43046c4. Read the comment docs.

Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together.

Apart from a couple of inline nitpicks, those code samples aren't particularly short. Maybe it would make sense to shorten the FAQ answer a lot, and link to a new recipe instead?

docs/user/faq.rst Outdated Show resolved Hide resolved
docs/user/faq.rst Outdated Show resolved Hide resolved
docs/user/faq.rst Outdated Show resolved Hide resolved
docs/user/faq.rst Outdated Show resolved Hide resolved
@CaselIT
Copy link
Member Author

CaselIT commented Oct 27, 2021

those code samples aren't particularly short

they are not, but I don't think we can shorten them more without making it difficult to follow.

Maybe it makes sense to have this here now, then we refactor this to a recepie, that includes a better version for wsgi, this one for asgi, both with also sqlalchemy option and maybe redis?

@CaselIT CaselIT requested review from vytas7 and kgriffs October 27, 2021 19:26
@vytas7 vytas7 changed the title Add a faq entry based on @vytas7 answer at docs(FAQ): add a FAQ entry based on @vytas7 discussion answer Nov 7, 2021
Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

I made a couple of minor tweaks, and this is now IMHO good enough to surface it in the docs.
We can always improve it later.

Once again, thanks for putting this together!

@vytas7 vytas7 merged commit cd1c555 into falconry:master Nov 7, 2021
@CaselIT CaselIT deleted the connection_faq branch November 7, 2021 14:33
@CaselIT
Copy link
Member Author

CaselIT commented Nov 7, 2021

Thanks!

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.

2 participants