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

Implemented client credential grant type #138

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

prakharjoshi
Copy link

Implemented client credential grant type without disturbing the default setting for authenticate code grant type

@prakharjoshi
Copy link
Author

related to #138

Copy link
Member

@mrichar1 mrichar1 left a comment

Choose a reason for hiding this comment

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

Hi,

Thanks for this PR. However there are a couple of 'tidying' things that would be needed before the code can be fully reviewed:

  • Please put the change comments into the commit message, not README.rst

  • Please update the documentation to explain this change.

  • from authomatic.providers import AuthorizationProvider as authprovider is redundant, given that providers.AuthorizationProvider is used elsewhere.

  • Please try to not make whitespace changes, or commented-out code unless they are specific to your PR. e.g. Including core.py seems unnecessary

  • There are some places where the code could be more DRY - e.g. oauth2.py lines 130 where the same variable value is set twice.

Hopefully these things can be cleaned up and we can then properly review your request!

@mrichar1
Copy link
Member

After several years of inactivity, authomatic is now under community management, and we have just released a new stable version (1.0.0).

We are now reviewing all issues and PRs and hoping to begin work to solve as many of these as possible.

We are keen to find out which issues still apply, and which PRs are still required/are likely to merge cleanly into the current code. We are aiming to review them all, but any help with prioritisation would be very useful!

If you are still interested in having this issue/PR resolved, or are able to help us work on it, please reply to this message. That way we know which issues are most important to the community.

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

2 participants