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

Sort third-party packages and add starlette-wtf #1415

Merged
merged 6 commits into from
Jan 16, 2022
Merged

Sort third-party packages and add starlette-wtf #1415

merged 6 commits into from
Jan 16, 2022

Conversation

aminalaee
Copy link
Member

According to discussion comment here adds starlette-wtf package to the docs.

@aminalaee
Copy link
Member Author

aminalaee commented Jan 15, 2022 •
edited
Loading

I can see the third-party packages can be improved. We can sort the packages alphabetically, the same as Third Party Middlewares, and move the middleware to Third Party Middlewares.

Maybe even include #1414 here as we'll move a few things around.

@aminalaee aminalaee changed the title Add starlette-wtf third-party package Sort third-party packages and add starlette-wtf Jan 16, 2022
@aminalaee
Copy link
Member Author

Right now we have Starlette Context, Starlette Cramjam and Starlette OAuth2 API in third party packages but they're middleware and should be moved to Third Party Middlewares.

@aminalaee aminalaee requested a review from a team January 16, 2022 14:23
adriangb
adriangb approved these changes Jan 16, 2022
View reviewed changes
docs/third-party-packages.md
@@ -3,7 +3,6 @@ Starlette has a rapidly growing community of developers, building tools that int

Here are some of those third party packages:


Copy link
Member

Choose a reason for hiding this comment

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

Should we maybe be using a markdown linter / formatter to control newlines and stuff like that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be great if we find one, otherwise we should just do it in PR review.

Copy link
Member

Choose a reason for hiding this comment

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

+1 for just doing it in PR review.
Not a big deal either ways, and it's not worth the extra tooling.

@aminalaee aminalaee merged commit e06425b into encode:master Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomchristie tomchristie tomchristie left review comments

@adriangb adriangb adriangb 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.
3 participants
@aminalaee @tomchristie @adriangb