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

TunesTakeout #5

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

TunesTakeout #5

wants to merge 37 commits into from

Conversation

jlikesplants
Copy link

Well, this was a doozy, as you know.

Things I'm proud of:
I spent a lot of time following the data flow through my app, using binding.pry to see at various moments what data was available.
I also think I got more comfortable with rails, though routes are still sometimes challenging.
I was able to standardize most of the search response data from the TunesTakeout API so that I could reuse methods and a partial to display them. This took a long time.
I did a better job this time at pulling logic into the models.

I think I could have figured out some tests with more time, but ended up giving up on that for the moment. I also realized I don't really know anything about bootstrap, so that's on the study list as well.

Thanks!


def new
@user
end

Choose a reason for hiding this comment

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

I don't understand what there is a variable here that isn't set to anything. Are you using this somehow? If so, set it to something. If not, you can get rid of it.

@rileslovesyall
Copy link

Overall, great job! I like how you creatively got your @data working so you could use that partial. It was confusing at first, but then I realized what was up. In general, just remember to go through and clean up your code before a PR (commented out stuff, spacing, etc). Good job keeping a separation of concerns between the Models and the Controllers, and good luck working on testing next time.

Cheers!

@jlikesplants
Copy link
Author

Thank you so much, Riley! I appreciate your feedback and your time. I felt
like I was just throwing the project at the wall by the time we were at
pull request due date, so it's awesome to have a moment to breathe and read
through your thoughts.

Thank you <3
Justine

On Tue, May 24, 2016 at 3:43 PM, Riley Spicer [email protected]
wrote:

Overall, great job! I like how you creatively got your @DaTa working so
you could use that partial. It was confusing at first, but then I realized
what was up. In general, just remember to go through and clean up your code
before a PR (commented out stuff, spacing, etc). Good job keeping a
separation of concerns between the Models and the Controllers, and good
luck working on testing next time.

Cheers!


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#5 (comment)

@rileslovesyall
Copy link

I think every project I finished at Ada felt like throwing the project at a wall by the end ;) You're doing a great job!

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