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

Initial implementation of Spotifiy API connection, including a test suite #18

Merged
merged 6 commits into from
Nov 16, 2020

Conversation

cloud-matt
Copy link
Member

This includes a JS based connection between our application and the Spotify API, allowing for easy development upon this structure.

Closes #9

After your review, we should meet to see the next steps for this project.

@cloud-matt cloud-matt added the enhancement New feature or request label Nov 10, 2020
@cloud-matt cloud-matt self-assigned this Nov 10, 2020
Copy link
Member

@kyrod kyrod left a comment

Choose a reason for hiding this comment

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

looks good, we can rework the tests in a future issue

src/js/GoldifyExecutePage.js Outdated Show resolved Hide resolved
src/js/GoldifyExecutePage.js Outdated Show resolved Hide resolved
@cloud-matt
Copy link
Member Author

Note: I made a small change to our CI workflow config file to include our Github Secrets as environment vars. Turns out it works! I wasn't sure if it was going to based on [https://github.com/actions/starter-workflows/issues/68](this issue thread).

Anyway, you may want to check that too to make sure it looks good. My apologies for editing it directly in master, that won't happen again.

@cloud-matt
Copy link
Member Author

For future reference:

  • Original Review was given approval as we were going to re-factor testing in the future. However, we came to the conclusion that we should get the testing situated now before we keep moving forward. Build a strong foundation.

I have since requested another review from @kyrod, who has much more experience with testing, as well as CI configuration (which I made a note about some changes above).

Kyle, if you see any changes that should be made, I say we get those in before we push through this PR. That way we're pretty concrete with our testing structure before reaaaally diving in. Thanks 💯

@cloud-matt cloud-matt added this to the Front End Only Complete milestone Nov 16, 2020
Copy link
Member

@kyrod kyrod left a comment

Choose a reason for hiding this comment

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

looks good to me bruther

@cloud-matt cloud-matt merged commit 8fab8a7 into master Nov 16, 2020
@cloud-matt cloud-matt deleted the initial-spotify-api-connection branch November 16, 2020 23:10
@cloud-matt
Copy link
Member Author

Closes #10

@cloud-matt cloud-matt linked an issue Nov 16, 2020 that may be closed by this pull request
@cloud-matt cloud-matt restored the initial-spotify-api-connection branch November 19, 2020 05:33
@cloud-matt cloud-matt deleted the initial-spotify-api-connection branch November 19, 2020 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users to login to personal Spotify Set-up Spotify API integration
2 participants