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

All Features and Pages Complete - No Testing #15

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

Conversation

nicosaki
Copy link

Finished all the features and functionality, but of the few tests I wrote, virtually none work. I have no idea how to test, as it turns out.
Standardizing the output from all of the APIs, which somehow had almost no standardization and contained all kinds of junk, was the worst. BOO spotify, you're even worse than yelp.

nicosaki added 30 commits May 16, 2016 15:55
…takeoutwrapper functionality around favorites'
@hougland
Copy link

Hey there! I'm Ricky, I'll be doing your code review for this project :)
Feel free to ask me if you need any clarification on any of my comments.

@@ -26,7 +27,7 @@
/tmp/

# Used by dotenv library to load environment variables.
# .env
.env

Choose a reason for hiding this comment

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

good job not checking your secrets in to github!

@hougland
Copy link

Overall, great job! It seems like this was a really complicated project, especially dealing with gems vs. wrapper, and funky models. Woohoo APIs!! 😄

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.

2 participants