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(tutorial): port the ASGI tutorial #1737

Merged
merged 38 commits into from
Dec 22, 2020

Conversation

vytas7
Copy link
Member

@vytas7 vytas7 commented Jul 5, 2020

Closes #1713

  • Create an ASGI tutorial skeleton, start porting from https://github.com/vytas7/falcon-asgi-example
  • Create a script to check that in-line snippets are in sync no longer needed
  • Set up basic asgilook Travis CI gate
  • Finish porting README.rst and all files
  • Run asgilook tests in GitHub Actions

@vytas7 vytas7 marked this pull request as draft July 5, 2020 22:32
@codecov
Copy link

codecov bot commented Jul 5, 2020

Codecov Report

Merging #1737 (7d347e3) into master (ba78a52) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1737   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           54        54           
  Lines         5128      5128           
  Branches       823       823           
=========================================
  Hits          5128      5128           

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 ba78a52...7d347e3. Read the comment docs.

@vytas7
Copy link
Member Author

vytas7 commented Dec 12, 2020

It's still a bit dull and repetitive, and I find things I'd like to change every time I reread it myself.
With that disclaimer in mind, the most glaring should now be addressed... Ready for review!

@vytas7 vytas7 marked this pull request as ready for review December 12, 2020 01:25
Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

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

Looking great 🚀 .

I think we can avoid the copy-paste of code alltogheter by using the sphinx directive literalinclude that does exactly that for us.

I think we could also update the wsgi tutorial to use it.

@kgriffs
Copy link
Member

kgriffs commented Dec 17, 2020

Type hints could be nice, but it does feel like a bit larger change, and it would probably make sense to make the same improvement to the WSGI tutorial at the same time.

Has an issue been created to track this?

@vytas7
Copy link
Member Author

vytas7 commented Dec 17, 2020

Has an issue been created to track this?

@kgriffs not specifically for tutorials and snippets, at least not yet.

We do have a generic typing issue though: #1350

@vytas7
Copy link
Member Author

vytas7 commented Dec 17, 2020

Now it has been created. 🙂 ⬆️ Thanks @CaselIT !

Copy link
Member

@kgriffs kgriffs left a comment

Choose a reason for hiding this comment

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

OK, I just found a few minor things that would be good to address for our first beta. Before the second beta, there are a handful of grammatical nits that I can circle back on.

@vytas7 vytas7 requested review from kgriffs, CaselIT and nZac December 19, 2020 00:56
CaselIT
CaselIT previously approved these changes Dec 19, 2020
Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

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

Looks good. Just a minor comment

Copy link
Member

@kgriffs kgriffs left a comment

Choose a reason for hiding this comment

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

Good improvements. Some grammatical odds and ends could be cleaned up to conform to the KGRIFFS Style Guide™ but we can circle back on those later.

@kgriffs kgriffs merged commit 9205f9c into falconry:master Dec 22, 2020
@vytas7
Copy link
Member Author

vytas7 commented Dec 22, 2020

to conform to the KGRIFFS Style Guide™

Heh, it seems that joke stuck 🤣 (Patent pending)

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.

quart to falcon
3 participants