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

Support multiple instances of one provider type #130

Merged
merged 4 commits into from Jan 27, 2017

Conversation

willemvd
Copy link
Collaborator

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 ?

… break implementing applications when they would call provider.Name() right now (do they?)
@willemvd
Copy link
Collaborator Author

No influence on #126

@markbates
Copy link
Owner

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.

@willemvd
Copy link
Collaborator Author

Think you end up with something like this

		provider := github.New(clientID, clientSecret, "http://localhost:3000/auth/github/callback")
		provider.Name = func() string {
			return "github" // or any other name
		}

but when you have multiple types of providers this might get messy with lots of extra func definitions.
That is why I have suggested this approach, but keeping Name() with some extra SetName() might do the trick

@willemvd
Copy link
Collaborator Author

I will update the PR with that approach

@markbates
Copy link
Owner

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.

@willemvd
Copy link
Collaborator Author

All the New methods set the default name in the Provider
example: https://github.com/willemvd/goth/blob/name-method/providers/amazon/amazon.go#L41

@willemvd
Copy link
Collaborator Author

Btw see a new conflict on intercom, will merge that

# Conflicts:
#	providers/intercom/intercom.go
@markbates
Copy link
Owner

Ah! Awesome! I missed that. Sorry.

@willemvd
Copy link
Collaborator Author

NP 😄

@markbates markbates merged commit b865c18 into markbates:master Jan 27, 2017
@willemvd willemvd mentioned this pull request Feb 2, 2017
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