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

any plan for a new major release? #378

Closed
eerison opened this issue Sep 30, 2022 · 9 comments
Closed

any plan for a new major release? #378

eerison opened this issue Sep 30, 2022 · 9 comments

Comments

@eerison
Copy link

eerison commented Sep 30, 2022

I was wondering if someone planned to release a new major to not keep all providers into the code base:
https://github.com/knpuniversity/oauth2-client-bundle/tree/master/src/Client/Provider

we could do something like oauth2-client does!

provide an interface and each provider implement that!

@eerison eerison changed the title any plan for new major? any plan for a new major release? Sep 30, 2022
@weaverryan
Copy link
Member

Hi!

No immediate plans for a new major - but just because we haven't seen any need yet.

to not keep all providers into the code base:

You think some should be removed? What would be the motivation for that?

provide an interface and each provider implement that!

Can you explain this a bit further? I'm not familiar with what oauth2-client does for this :).

Cheers!

@eerison
Copy link
Author

eerison commented Oct 4, 2022

Hi @weaverryan

I was thinking to follow the same idea as oauth2-client, if you check the providers source it doesn't implement all possible providers.

it just depends one interface and that is it!

IMO we should do something like this.

my idea is create a repository with the interfaces that we need and the providers can create a new client to follow the interface or just implement the interface that!

if we do it, we don't need to add a lot of providers in our code base!

@eerison
Copy link
Author

eerison commented Oct 4, 2022

For example was is the basics fields that is needed to make this bundle works?

firstName, lastName and Email? if yes then we could create interfaces for this and for providers support this bundle needs to implement those interfaces

@weaverryan
Copy link
Member

Ah, I think I understand. The idea behind this bundle was actually to purposefully put the small amount of "provider glue code" inside of this repository to make everyone's live easier. I think you're proposing a structure that would have the following libraries (using facebook as an example):

A) oauth2-client
B) league/oauth2-facebook (the client provider for oauth2-client + Facebook)
C) knpuniversity/oauth2-client-bundle
D) some new oauth2-facebook-client which would be a "provider" to glue league/oauth2-facebook together with knpuniversity/oauth2-client-bundle

If that's accurate, I think it's more realistic to maintain one bundle (this one) that has all of that "glue code" inside of it. If we don't provide that glue code, in reality, people will not create entire separate libraries to implement that part. If you mean that libraries like league/oauth2-facebook could add 1 class of glue code, again, I don't think that will happen in practice: many libraries won't want Symfony-specific classes in their libraries.

Cheers!

@eerison
Copy link
Author

eerison commented Oct 4, 2022

If you mean that libraries like league/oauth2-facebook could add 1 class of glue code

the league/oauth2-facebook doesn't need to depends of knpuniversity/oauth2-client-bundle we could create a repository only with contracts like https://github.com/symfony/http-client-contracts and league/oauth2-facebook could add this contract library into the dependencies.

after we have an interface in common we can depends just of this interface!

@eerison
Copy link
Author

eerison commented Oct 4, 2022

because the biggest problem is, the providers doesn't return a common interface with

firstName, lastName and Email (maybe with ImagePath as optional), and oauth2-client doesn't provide those common interface.

I tried to suggest it there, But anyone answer me :'(

thephpleague/oauth2-client#958

then an a second possible solution is, create this small repository with those interfaces and the providers that return it, can be supported by this bundle.

@eerison
Copy link
Author

eerison commented Oct 6, 2022

@weaverryan what do you think? should we invest time on that, or let's keep as it's?

@weaverryan
Copy link
Member

Hi!

could create a repository only with contracts like https://github.com/symfony/http-client-contracts and league/oauth2-facebook could add this contract library into the dependencies.

That would need to be something done in coordination with with oauth2-cilent itself, I think. If someone external creates this repository, I don't think they will integrate with it.

@weaverryan what do you think? should we invest time on that, or let's keep as it's?

If there is something that can be done in this repo, without creating an external repo or needing up-stream libraries like oauth2-facebook to do something, I'm open to it :). But I'm not sure that's what you're proposing.

@eerison
Copy link
Author

eerison commented Oct 6, 2022

I think. If someone external creates this repository, I don't think they will integrate with it.

Yeah it's true :/
Let's see if someone reply on oauth2-cilent, if yes then I can raise up this again ;)

@eerison eerison closed this as completed Oct 6, 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

No branches or pull requests

2 participants