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

document and refactor the handlers #183

Merged
merged 4 commits into from
Nov 2, 2021
Merged

document and refactor the handlers #183

merged 4 commits into from
Nov 2, 2021

Conversation

lambdaTotoro
Copy link
Collaborator

I did a thorough pass on the handlers and did some documenting and refactoring in a few places (for example, we now no longer have "AuthorizeHandler", "AuthorizationHandler", and "ChangeAuthorizationHandler" that all do vastly different things).

Also simplified the logic on error reporting in a few places.

Ideally, I would hope that this closes #177.

@lambdaTotoro lambdaTotoro added enhancement New feature or request documentation labels Nov 2, 2021
@lambdaTotoro lambdaTotoro added this to the Version 1.1 milestone Nov 2, 2021
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.

This looks great! This makes such a big difference! Wieeeeeeeee!!!

I had some tweak requests if i recall correctly, but this looks great!

nativeauthenticator/handlers.py Outdated Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Outdated Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
nativeauthenticator/handlers.py Outdated Show resolved Hide resolved
nativeauthenticator/handlers.py Show resolved Hide resolved
@consideRatio consideRatio added maintenance and removed enhancement New feature or request labels Nov 2, 2021
@consideRatio
Copy link
Member

I relabelled this as documentation/maintenace rather than documentation/enhancement -- which i consisder would be a feature that matters to the end user.

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
@lambdaTotoro
Copy link
Collaborator Author

I have resolved everything sans the slug debate. There, I have included some code comments as well, see if this is sufficient in your opinion.

@consideRatio consideRatio merged commit fe3fbe6 into main Nov 2, 2021
@consideRatio
Copy link
Member

Beautiful @lambdaTotoro! I think this is crucial work for someone like me who aren't that familiar with the code base or that haven't tried all the features, such as email-authorization.

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.

Add docstring for handlers
2 participants