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

Type subreddit #1587

Merged
merged 6 commits into from
Feb 18, 2021
Merged

Type subreddit #1587

merged 6 commits into from
Feb 18, 2021

Conversation

PythonCoderAS
Copy link
Contributor

This is the typed version of subreddit.py

@PythonCoderAS PythonCoderAS added this to the Type all Modules milestone Nov 29, 2020
Copy link
Contributor

@jarhill0 jarhill0 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 to me! There was just one thing I was unsure about.

from urllib.parse import urljoin
from xml.etree.ElementTree import XML

import websocket
from prawcore import Redirect
from requests import Response
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like Response is only used in a type signature. Should this be under if TYPE_CHECKING:?

I'm not very familiar with the intricacies of Python's typing, so I'm just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I imported Response because it doesn't interfere with the import chain. I put under if TYPE_CHECKING anything that could cause a circular import loop, or anything else that I imported from PRAW. However, I highly doubt a third-party package would form such a loop.

@LilSpazJoekp
Copy link
Member

Could you resolve conflicts?

@LilSpazJoekp
Copy link
Member

Could you please run pre-push again? It looks black is having issues with it.

@LilSpazJoekp
Copy link
Member

Could you rebase the changes from master instead of merge please?

@PythonCoderAS
Copy link
Contributor Author

Can you merge and squash?

@LilSpazJoekp
Copy link
Member

I would prefer if you did a rebase. Squash and merge doesn't create a merge commit.

@LilSpazJoekp
Copy link
Member

LilSpazJoekp commented Feb 10, 2021

Could you rebase and squash please?

@LilSpazJoekp
Copy link
Member

I'll get this finished up for you.

@LilSpazJoekp
Copy link
Member

This PR is dependent on #1681 being merged.

@LilSpazJoekp LilSpazJoekp merged commit ab3dffb into praw-dev:master Feb 18, 2021
@PythonCoderAS PythonCoderAS deleted the type-subreddit branch February 24, 2021 15:16
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.

None yet

3 participants