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.

Sign up for GitHub

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

'Dependencies' chapter fixes #1675

Merged
merged 13 commits into from
Aug 3, 2020
Merged

'Dependencies' chapter fixes #1675

merged 13 commits into from
Aug 3, 2020

Conversation

nilslindemann
Copy link
Contributor

No description provided.

@nilslindemann nilslindemann changed the title Add missing word Misc small fixes Jul 5, 2020
@nilslindemann
Copy link
Contributor Author

Notice, I'm just reading through the docs, will maybe add more fixes while doing so. Please don't merge immediately. Will write another message when I have read through the tutorial, ok? Then you can merge (if you like my changes ;-) )

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jul 5, 2020

You can add a flag WIP: on the PR title for the next PRs :)

@phy25
Copy link

phy25 commented Jul 5, 2020

"Changing the stage of a pull request - GitHub Docs" https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-stage-of-a-pull-request

@nilslindemann nilslindemann changed the title Misc small fixes WIP: Misc small fixes Jul 5, 2020
@nilslindemann
Copy link
Contributor Author

nilslindemann commented Jul 5, 2020 •
edited
Loading

Thx for the WIP: ('Work in progress') hint, @Kludex

@nilslindemann
Simplify sentence
555b9be
@nilslindemann
Grammar
796318e 
The verb 'help' is used with an infinitive: 'to avoid' or 'avoid'.
@nilslindemann
Typo
5f96a98 
The verb re-use can be separated with a dash but not the adjective unused (https://en.wiktionary.org/wiki/unused). I guess that is because there is no verb "to un(-)use".
@nilslindemann nilslindemann changed the title WIP: Misc small fixes Fixes in 'dependencies' chapter Jul 5, 2020
@nilslindemann
Copy link
Contributor Author

Ok, im done with the dependencies chapter.

I will create new branches for other chapters in order to not make this PR too big or prevent other people from making changes.

Can be merged (if there is interest doing so).

@nilslindemann
Copy link
Contributor Author

nilslindemann commented Jul 6, 2020 •
edited
Loading

@phy25 i misread your comment yesterday (that you made some changes).

Thanks for this link, but i dont see such merge buttons, My own repo has no pull requests (obviously). I see this and other PRs in this repo, but when i click mine, i come here, there is no merge button.

Edit: i figured it out. Press the 'Convert to draft' link top right first. Once again GitHub shows its nerdiness by explaining how to deactivate an (invisible) feature before explaining how to actually activate it.

@nilslindemann nilslindemann changed the title Fixes in 'dependencies' chapter 'dependencies' chapter fixes Jul 6, 2020
@nilslindemann nilslindemann changed the title 'dependencies' chapter fixes 'Dependencies' chapter fixes Jul 6, 2020
@nilslindemann
Reformulation
dd01332 
Put the sentence after the code example.

Remove the sentence 'You can declare that parameter `commons` to be of type of the class, `CommonQueryParams`.' because this is explained directly below.

Remove the horizontal ruler and replace 'In this case, ' with 'but'.
@nilslindemann
Radical shortening of the Type annotation vs Depends chapter
bec43cc 
I hope it is not too heretic :-)

This makes dependencies/tutorial003.py redundant
@nilslindemann
Copy link
Contributor Author

I added one more change. Let me know if this is too much.

@codecov
Copy link

codecov bot commented Aug 3, 2020 •
edited
Loading

Codecov Report

Merging #1675 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##            master     #1675    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          236       235     -1     
  Lines         7150      6989   -161     
==========================================
- Hits          7150      6989   -161
Impacted Files Coverage Δ
fastapi/applications.py 100.00% <0.00%> (ø)
fastapi/openapi/utils.py 100.00% <0.00%> (ø)
fastapi/security/http.py 100.00% <0.00%> (ø)
fastapi/security/oauth2.py 100.00% <0.00%> (ø)
fastapi/dependencies/utils.py 100.00% <0.00%> (ø)
tests/test_deprecated_openapi_prefix.py 100.00% <0.00%> (ø)
...s/test_tutorial/test_templates/test_tutorial001.py 100.00% <0.00%> (ø)
.../test_tutorial/test_websockets/test_tutorial001.py 100.00% <0.00%> (ø)
.../test_tutorial/test_websockets/test_tutorial002.py 100.00% <0.00%> (ø)
...t_tutorial/test_behind_a_proxy/test_tutorial001.py 100.00% <0.00%> (ø)
... and 9 more

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 3ff504f...d7f7589. Read the comment docs.

@tiangolo tiangolo merged commit 9840d9e into tiangolo:master Aug 3, 2020
@tiangolo
Copy link
Owner

tiangolo commented Aug 3, 2020

Great! 🔍

I reverted a couple of changes to keep the same explicitness and clarity, etc.

Thanks for your contribution! ☕ 🍰

And thanks for the help @phy25 and @Kludex !

@nilslindemann nilslindemann deleted the patch-1 branch August 3, 2020 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kludex Kludex Kludex 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
4 participants
@nilslindemann @Kludex @phy25 @tiangolo