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

Test improvements and upgrade pre_push script #126

Merged
merged 7 commits into from Jan 4, 2022
Merged

Conversation

LilSpazJoekp
Copy link
Member

No description provided.

prawcore/auth.py Outdated
@@ -64,8 +64,7 @@ def authorize_url(self, duration, scopes, state, implicit=False):
)
if implicit and duration != "temporary":
raise InvalidInvocation(
"The implicit grant flow only supports "
"temporary access tokens."
"The implicit grant flow only supports " "temporary access tokens."
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it would appear so.

Copy link
Contributor

@vikramaditya91 vikramaditya91 left a comment

Choose a reason for hiding this comment

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

Looks very good. Minor suggestions/changes.

Would be also interesting to add flake8-docstrings and flake8-annotations. But this branch is already doing too mcuh, so we could do that in a new branch later

prawcore/auth.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
pre_push.py Show resolved Hide resolved
pre_push.py Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@vikramaditya91
Copy link
Contributor

Alright. Looks good. I do not have permission to approve but no further comments from my side.

@LilSpazJoekp
Copy link
Member Author

Thank you, I am waiting for @bboe's approval before I merge.

@CAM-Gerlach
Copy link
Contributor

Seems like this PR stalled, but to greatly simplify maintenance, now that praw-dev/praw#1784 is merged and this has apparently picked up merge conflicts (GitHub won't show me them for some reason), it might be a good idea to just re-sync with that, and in the future, all that will be necessary to keep them in sync is pre-commit autoupdate/using a regular pre-commit autoupdate action, and occasionally syncing any important changes in the pre-commit config itself.

@LilSpazJoekp LilSpazJoekp force-pushed the pre_push branch 2 times, most recently from 42835df to 5676c97 Compare January 4, 2022 05:37
@LilSpazJoekp LilSpazJoekp changed the title Upgrade pre_push script Test improvements and upgrade pre_push script Jan 4, 2022
@LilSpazJoekp LilSpazJoekp merged commit d45ef8e into main Jan 4, 2022
@LilSpazJoekp LilSpazJoekp deleted the pre_push branch January 4, 2022 06:34
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