-
Notifications
You must be signed in to change notification settings - Fork 146
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
Change underlying library for discord because it's not compliant with oauth2-client v2.x #90
Conversation
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.
In src/Client/Provider/DiscordClient.php
the namespace of Discord\OAuth\Parts\User
probably also should be updated.
README.md
Outdated
@@ -66,7 +66,7 @@ via Composer: | |||
| [Clever](https://github.com/schoolrunner/oauth2-clever) | composer require schoolrunner/oauth2-clever | | |||
| [DevianArt](https://github.com/SeinopSys/oauth2-deviantart) | composer require seinopsys/oauth2-deviantart | | |||
| [DigitalOcean](https://github.com/chrishemmings/oauth2-digitalocean) | composer require chrishemmings/oauth2-digitalocean | | |||
| [Discord](https://github.com/teamreflex/oauth2-discord) | composer require team-reflex/oauth2-discord | | |||
| [Discord](https://github.com/wohali/oauth2-discord-new) | composer require team-reflex/oauth2-discord | |
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.
Should be "... require wohali/oauth2-discord-new" in the end, you changed the link URL only
On one hand, it makes sense to move Discord to the OAuthClient v2, but on the other - we may provide a BC break with this PR. Probably to avoid BC breaks we need to keep both? Or, maybe a bit complicate the logic with some extra checks like if the new class from |
I fixed your comments. good catch. Also, the edits from maintainers are activated if you want to provide any kind of fallbacks (which is overrated imo) |
Thanks @tristanbes! And yea, I agree - this is a small BC break, but I also really don't want to deal with fallback logic. Hopefully this type of thing will be rare. |
Maybe just add it in UPGRADE or Readme to tell user that they need to require the new lib instead of the previous one. And everything will work the same ;-) My pleasure. |
Because the previous library is only compatible with the ultra old
thephpleague/oauth2-client
in version1.x
.The documentation no longer reference the library
https://github.com/teamreflex/oauth2-discord
. Instead, they promote https://github.com/wohali/oauth2-discord-new.See: http://oauth2-client.thephpleague.com/providers/thirdparty/