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

maint: Ditch dependency to OAuthenticator #164

Merged
merged 9 commits into from
Jun 16, 2023

Conversation

martinclaus
Copy link
Collaborator

Closes #155

@martinclaus martinclaus marked this pull request as draft June 6, 2023 08:08
@martinclaus martinclaus marked this pull request as ready for review June 6, 2023 13:44
@martinclaus martinclaus changed the title Ditch dependency to OAuthenticator maint: Ditch dependency to OAuthenticator Jun 13, 2023
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

I didn't review the test code changes, but this looks great from what I can tell.

If you have verified this to work, 👍 for merging without further ado!

I appreciate that you have included docstrings for small functions as well, for example compare value in cookie with redirect url param, I love it!!


Oh, merge conflicts to resolve, but seems trivial.

@martinclaus
Copy link
Collaborator Author

I successfully tested this PR on a local deployment.

@martinclaus martinclaus merged commit de52f3e into main Jun 16, 2023
15 checks passed
@martinclaus martinclaus deleted the ditch-oauthenticator-dependency branch June 16, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do we really need to depend on oauthenticator?
2 participants