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): add new tutorial covering core features #769

Merged
merged 45 commits into from
Jan 15, 2022

Conversation

mploski
Copy link
Contributor

@mploski mploski commented Oct 18, 2021

#580

Description of changes:

Expanding Readme.md file with new Quickstart section
Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


View rendered README.md

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 18, 2021
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Oct 18, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 18, 2021

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

@mploski mploski requested a review from heitorlessa October 18, 2021 21:18
@mploski mploski changed the title Add quickstart section in Readme.md docs: Add quickstart section in Readme.md Oct 18, 2021
@mploski mploski changed the title docs: Add quickstart section in Readme.md docs: add quickstart section in Readme.md Oct 18, 2021
@DanyC97
Copy link
Contributor

DanyC97 commented Oct 21, 2021

@mploski thanks for looking into this, do you have any thoughts/ have you considered how this new block will compare with https://github.com/aws-samples/cookiecutter-aws-sam-python which is also in the current Readme file?

where i'm coming from is:

As a new user i should have a single place where i can go and see an abundance of examples (in various phases of complexity starting from simple one to more like real projects) so it can be easy and confusion free.

@mploski
Copy link
Contributor Author

mploski commented Oct 22, 2021

@mploski thanks for looking into this, do you have any thoughts/ have you considered how this new block will compare with https://github.com/aws-samples/cookiecutter-aws-sam-python which is also in the current Readme file?

where i'm coming from is:

As a new user i should have a single place where i can go and see an abundance of examples (in various phases of complexity starting from simple one to more like real projects) so it can be easy and confusion free.

@DanyC97 Thanks for the valid suggestion. I will adjust my PR to use cookiecutter example.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2021

Codecov Report

Merging #769 (e5e23d1) into develop (24ab726) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #769   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5320     5320           
  Branches       613      613           
========================================
  Hits          5318     5318           
  Partials         2        2           

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 24ab726...e5e23d1. Read the comment docs.

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Thanks a lot @DanyC97 for helping us improve, and @mploski for taking this head on!

A few tips:

  • Tracing instead of X-Ray Tracing
  • Break some paragraphs so it’s easier to read
  • Make use of Admonitions to add banners like Tips, Info, etc.
  • EMF and Log Code snippet is a single line instead of being multi-line
  • Share ideas on what customers could do besides what’s being explained - take the opportunity to teach customers tips on how to make the best out of practices
    • Examples of tracing annotations vs metadata
    • Ideas for custom metrics metadata so customers can quickly search info related to metrics
  • Share some notes on the Infra being used, env vars, etc.
  • Don’t be afraid of using bold, bullet points, etc. strike a balance between being sparse and opinionated
  • Add links to the AWS Console when you mention - X-RAY Console - to help customers find that themselves too

Because there are many customers new to Python and developing on AWS Lambda, this is a great opportunity for us to share more general tips as they go along the quickstart too... until we have a public workshop covering these in more depth.

@heitorlessa
Copy link
Contributor

Adding Pablo as a tentative reviewer since he's gone through some hurdles in his first experience that we can improve it.

@mploski
Copy link
Contributor Author

mploski commented Oct 29, 2021

@heitorlessa Thanks for the feedback! As discussed offline I will change the approach and build the "progressing feeling" where user build final hello-world app from the ground up, adding new functionalities as they go up to the state we have/will have in here https://github.com/aws-samples/cookiecutter-aws-sam-python/blob/master/%7B%7B%20cookiecutter.project_name%20%7D%7D/hello_world/app.py

@heitorlessa heitorlessa assigned mploski and unassigned pcolazurdo Nov 11, 2021
@heitorlessa heitorlessa marked this pull request as draft November 14, 2021 13:08
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 28, 2021
@mploski mploski requested a review from heitorlessa November 29, 2021 08:10
@mploski mploski marked this pull request as ready for review November 30, 2021 15:14
@mploski
Copy link
Contributor Author

mploski commented Nov 30, 2021

@heitorlessa @DanyC97 Please re-review if possible :-) Thanks!

Copy link
Contributor

@am29d am29d left a comment

Choose a reason for hiding this comment

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

This is a really comprehensive quickstart, great work @mploski !

I have added some comments but here are few general points:

  • reduce the sentence length and the number of sentences per block. It's hard to keep people's attention so we don't want to lose them.
  • Turn into positive. In some parts you present disadvantages and then introduce Powertools as a solution. While it's in general true, it might have some negative tone. Try to convert it to positive expression and show how Powertools can make it easier. You changed your style in the last part of the docs, so it's already there, let's make it consistent.
  • "You have a task to do X..." it's a personal taste, but maybe a more guiding style would be approachable with "We want to do X, let's see how it works..."
  • Few grammar nitpicks. I am also not a native speaker, so maybe we can run it through some tool and double check.

docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
docs/quickstart.md Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 15, 2022
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 15, 2022
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 15, 2022
@heitorlessa heitorlessa changed the title docs: add quickstart section in Readme.md docs(tutorial): add new tutorial covering core utilities Jan 15, 2022
@heitorlessa heitorlessa changed the title docs(tutorial): add new tutorial covering core utilities docs(tutorial): add new tutorial covering core features Jan 15, 2022
@heitorlessa
Copy link
Contributor

heitorlessa commented Jan 15, 2022

@mploski All changes are now in this PR, please have a look and feel free to merge if you agree with them.

I largely created sub-sections, made all code snippets consistent, copywriting, suggestions where they can go next at the end of each section, added a Final considerations section, and due to the size renamed from Quickstart to Tutorial ;D

image

@mploski
Copy link
Contributor Author

mploski commented Jan 15, 2022

@mploski All changes are now in this PR, please have a look and feel free to merge if you agree with them.

I largely created sub-sections, made all code snippets consistent, copywriting, suggestions where they can go next at the end of each section, added a Final considerations section, and due to the size renamed from Quickstart to Tutorial ;D

image

It looks really great after all your changes. Thanks a million. Reviewed it, tweaked small things ( typos, making code a bit more consistent + fixing some lines references). I will merge it now :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants