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

Prefer Local Over Global Configuration #95

Merged
merged 1 commit into from
Sep 23, 2022
Merged

Prefer Local Over Global Configuration #95

merged 1 commit into from
Sep 23, 2022

Conversation

aj-foster
Copy link
Contributor

Hello 👋🏼

This PR is primarily for discussion purposes, with code as a demonstration.

In the current implementation, if someone configures this provider using the global configuration shown in the README, they cannot then provide a different configuration at the call site. This is because the library overrides local / passed-in configuration options with the global config.

In my opinion, the library should prefer local over global [over the defaults]. This way, different call sites can provide their own options while still falling back to the global config. We intend to use this at CodeSandbox to provide logins via multiple Google OAuth clients — one which is a clear default, and others which are specialized and locally configured.

Always open to feedback and discussion. Thanks!

@aj-foster aj-foster requested a review from a team as a code owner September 8, 2022 00:26
@yordis
Copy link
Member

yordis commented Sep 23, 2022

It makes sense that you could overwrite things and fallback to globals. Thank you for your contribution.

@yordis yordis merged commit d2f890c into ueberauth:master Sep 23, 2022
@aj-foster aj-foster deleted the draft/gracious-andras branch September 23, 2022 17:01
silvagustin added a commit to silvagustin/ueberauth_google that referenced this pull request Oct 14, 2022
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.

2 participants