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

JLN - Project Complete #37

Open
wants to merge 38 commits into
base: jln/master
Choose a base branch
from
Open

Conversation

noglows
Copy link

@noglows noglows commented Dec 4, 2015

Completed all requirements and styling.

100% Rspec test coverage.

@brittinator brittinator self-assigned this Dec 22, 2015

resources :movies
resources :books
resources :albums

Choose a reason for hiding this comment

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

Nice use of resources, you really did use all of them!

@brittinator
Copy link

Nice job! A couple last notes:

  • Your code between controllers is very consistent and makes it easy to follow - I like it.
  • I'm confused why you have _form, _index, _show and index all in 2 locations - application and shared. Are you using both versions of these?
  • I would add /coverage to your .gitignore. That way the huge files that change every time you run spec won't be tracked.
    I like your use of case in your shared example specs. Well done getting 100% coverage, that's something to celebrate for sure. 💯

{name: "A Mango Shaped Space", author: "Wendy Mass", description: "A thirteen-year-old girl living with synesthesia, a jumbling of the senses", votes: 0},
{name: "Where the Red Fern Grows", author: "Wilson Rawls", description: "A boy who buys and trains two Redbone Coonhound hunting dogs", votes: 0},
{name: "Ender's Game", author: "Orson Scott Card", description: "In preparation for an anticipated third invasion, children are trained from a very young age through increasingly difficult games", votes: 0}
]

Choose a reason for hiding this comment

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

Did you see the movie? I thought it was decent.

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