-
Notifications
You must be signed in to change notification settings - Fork 592
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
Support multiple instances of one provider type #130
Conversation
… break implementing applications when they would call provider.Name() right now (do they?)
No influence on #126 |
I'm not ok with a breaking API change. There needs to be a different way of doing this. Perhaps a setter? Or people could just wrap the provider in a type that reimplements the name function. |
Think you end up with something like this
but when you have multiple types of providers this might get messy with lots of extra func definitions. |
I will update the PR with that approach |
So my problem now is none of the providers have a default name. We need to return a default name for each provider unless one is set. If this PR was merged it would break existing apps, unless I'm missing something. |
All the New methods set the default name in the Provider |
Btw see a new conflict on intercom, will merge that |
# Conflicts: # providers/intercom/intercom.go
Ah! Awesome! I missed that. Sorry. |
NP 😄 |
To be able to register the same provider twice we need to be able to override the provider name.
Don't want this to be a new func registered to the provider.Name but be inline with Client ( #123 )
Downside is that existing calls to provider.Name() will break (easy to replace with GetName() )
This will maybe also fix issue #126 ?