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

Add discord as a new provider #248

Merged
merged 5 commits into from Sep 7, 2016
Merged

Conversation

@klarstil
Copy link
Contributor

klarstil commented Aug 4, 2016

I added discord (https://discordapp.com/developers/docs/topics/oauth2) as a new provider for bell. I'm working on a project which needs the ability to login with discord.

The tests are working & the code coverage is still 100%, here's a pastebin for the test results: http://pastebin.com/CA7GnBdV.

The documentation of the provider was added to the Providers.md file as well.

Let me know if there's something missing.

get('https://discordapp.com/api/users/@me', null, (profile) => {

credentials.profile = {
id: parseInt(profile.id),

This comment has been minimized.

Copy link
@ldesplat

ldesplat Aug 20, 2016

Contributor

I am not sure I agree with the parseInt. If they return this id as a string we should honor that and not throw when they don't!

@ldesplat ldesplat added the feature label Aug 20, 2016
@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Aug 20, 2016

I apologize for the delay. If you agree with that one review comment, then I can merge this.

@klarstil

This comment has been minimized.

Copy link
Contributor Author

klarstil commented Aug 29, 2016

Hey @ldesplat, no worries I was busy too. I updated the PR according to your CR & fixed a typo in the Providers.md.

klarstil added 2 commits Aug 29, 2016
@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Sep 7, 2016

Looks great. Thank you so much!

@ldesplat ldesplat merged commit c3fb208 into hapijs:master Sep 7, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ldesplat ldesplat added this to the 8.1.2 milestone Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.