-
-
Notifications
You must be signed in to change notification settings - Fork 254
Refactor Providers to be a singleton
#1327
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
Conversation
rmartinoscar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary as a provider? I don't think the majority of people are going to add anymore custom providers or if it's worth supporting them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Provider makes sense here since it configures the AvatarService. We could hardcode those 2 schemas as the only options the service supports, but that seems like it makes it less modular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary as a provider? I don't think the majority of people are going to add anymore custom providers or if it's worth supporting them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the AvatarService Provider.
| /** | ||
| * @return array<string, string|string[]|bool|null> | ||
| */ | ||
| public function getServiceConfig(): array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we refactor this to be either a not complex array (string[] only) or a dto?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Socialite wants different configs for different providers (Steam wants different configs than Gitlab).
Do you have any suggestions? Not sure how to make a DTO that's not just a bag of dynamic props.
Uh oh!
There was an error while loading. Please reload this page.