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

Queues - Marisol Lopez - API Muncher #65

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

Conversation

marisol-lopez
Copy link

API Muncher

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How did you go about exploring the Edamam API, how did you try querying the API?
Describe your API Wrapper. How did you decide on the methods you created?
Describe an edge case or failure case test you wrote for your API Wrapper.
Explain how VCR aids in testing an API.
What is the Heroku URL of your deployed application?
Do you have any recommendations on how we could improve this project for the next cohort?

@droberts-sea
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions no :(
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML needs improvement - I see lots of <div>s, many of which could probably be something more semantic like <section> or <article>. Also, in application.html.erb your page should be organized into <header>, <main> and <footer> sections.
Errors are reported to the user no - This should be one of the first things you do when you set up a new project (for example by adding code to display flash messages to application.html.erb). It will save you a lot of trouble in the long run.
API Wrapper to handle the API requests yes - see inline comment
Controller testing some - positive cases are well covered. Questions I would like to see your tests asking: "what happens if my search term returns 0 results?", "what happens if I try to show a recipe that doesn't exist?"
Styling
Foundation Styling for responsive layout yes
Search View yes
List View yes
Show View which opens in a new tab yes
API Features
The App attributes Edaman no - This is one of the conditions for using their API! You should have some their logo in your site's footer, as well as something like "Powered by Edamam".
The VCR casettes do not contain the API key yes
Overall

Good work overall!

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