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

optional Authorization Code Flow #8

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

moooji
Copy link

@moooji moooji commented Sep 23, 2016

No description provided.

@moooji moooji mentioned this pull request Sep 23, 2016
@digitaldavenyc
Copy link

@moooji This is awesome. I'm going to test it tomorrow and will let you know if there are any issues. I do have some concerns because there is an outstanding bug #6 that could have implications on this PR. Further changes may be required after that bug is fixed.

@digitaldavenyc
Copy link

Is this intended to be a starting point for using the beta 23 sdk? Including it sets off a lot of errors on other functions not related to auth.

@moooji
Copy link
Author

moooji commented Sep 24, 2016

No this PR should not have to do anything with SDK 23. As you can see in the changes, I have only added "tokenSwapURL" and "tokenRefreshURL" in spotifyModule/ios/SpotifyAuth.m. This should compile without problems with SDK 17.

@moooji
Copy link
Author

moooji commented Sep 24, 2016

I have also renamed SpotifyModule -> SpotifyAuth in README, because SpotifyModule does not exist and seem to be some legacy code?

Copy link
Owner

@viestat viestat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like using an object with options for Auth, awesome job!
I need you to change a couple things before I merge this. it would be nice to only set tokenSwapURL and tokenRefreshURL only if the user provides them, so maybe add a condition for that.
In the README file, please change back SpotifyAuth to SpotifyModule as I will change it in the code too.
Please forgive the late review but I have been super busy this past month.
Thank you so much for your contributions and your patience!

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.

3 participants