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

Don't access oauth2_settings if OAuthLibCore server parameter is prov… #1394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

claytonsingh
Copy link

@claytonsingh claytonsingh commented Jan 29, 2024

Description of the Change

Dont access oauth2_settings if server parameter is provided

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@n2ygk
Copy link
Member

n2ygk commented Feb 27, 2024

@claytonsingh what is the purpose of this change? How and why would one do this?

@claytonsingh
Copy link
Author

I have some specific requirements and am not using this library in INSTALLED_APPS. The methods and classes do provide extensive value though.

Copy link
Member

@n2ygk n2ygk left a comment

Choose a reason for hiding this comment

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

Please add a use case example to the documentation and add a test.

I can't merge something that doesn't have an clear (to others) purpose.
Thanks.

@dopry
Copy link
Contributor

dopry commented Oct 2, 2024

@claytonsingh bump, do you have time to wrap up this PR? Id be good to get the use case documented and some tests around the feature.

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.

3 participants