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

Tunes&Takeout, Val edition! #18

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

Conversation

digivava
Copy link

Here you go!
I wish I had had some time to style it more and play with UI/UX but.... this'll do!
I feel like I learned A LOT in this project. And there were a lot of challenges but they were all the kinds of things I feel like I'll run into on the job. So I feel like I leveled up in my docs-searching/stack-trace-reading/Google-fu a bit. Yay!

Test coverage still isn't what it could be, sadly. Sorry about that.. that's the one requirement that I didn't quite get done.

digivava added 30 commits May 16, 2016 16:29
…accidentally put them in the user_test.rb but I moved them, hopefully they are in the right spot now.
…ged-in users and guests differently, and added flash message to tell the user when they failed to login
… so trying now to decipher it and figure out why it's not working
…od. The greeting box at the top of every screen now accurately reflects if you are logged in or not.
…rs can see their spotify avatar in the greeting box
digivava added 17 commits May 20, 2016 11:30
…estions array to retrieve a random suggestion, instead of always the first one in the list
…s that suggestion to Faved when the index loads again. But user favorites page now broken.
…le confused by what all was going on there but oh well.

def new

end
Copy link

@dezshino dezshino May 24, 2016

Choose a reason for hiding this comment

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

Empty controller methods like this can be re-written in one line like: def new; end

@dezshino
Copy link

Great job on this project! APIs are a huge new concept - even if you weren't able to get the test coverage as high as you wanted, I think it's good you prioritized applying the new information. I don't see a heroku link for this, but I think I heard that wasn't required so it's all good. It's really great the amount you could do with this new topic in a short time frame!

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