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

Jessica Owens -- Carets #40

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

Jessica Owens -- Carets #40

wants to merge 31 commits into from

Conversation

vertige
Copy link

@vertige vertige commented Nov 6, 2017

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? Looked at their documentation (simple but maybe not super robust). Played around in Postman.
Describe your API Wrapper. How did you decide on the methods you created? I put my search methods for the API in there along with quite a few constants.
Describe an edge case or failure case test you wrote for your API Wrapper. I wrote a test for checking if the required information to create a Recipe instance weren't present.
Explain how VCR aids in testing an API. It helps us not blow our 5 calls/minute API restriction LOL. It stores API responses for specific interactions, so we save time and calls.
What is the Heroku URL of your deployed application? https://findmetherecipe.herokuapp.com/

@CheezItMan
Copy link

API Muncher

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and good commit messages
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) Check
Semantic HTML Check, nice use of a partial for the flash notice.
Errors are reported to the user Check
API Wrapper to handle the API requests Check
Controller testing Check
Lib testing Very through testing of your wrapper.
Search Functionality Check
List Functionality Check
Show individual item functionality (link to original recipe opens in new tab) The link is there, it doesn't open in a new tab
Styling
Foundation Styling for responsive layout Check
List View shows 10 items at a time/pagination Check
The app is styled to create an attractive user interface Check, although the default text in the search bar annoyed the hell out of me.
API Features
The App attributes Edaman Check
The VCR casettes do not contain the API key Check
External Resources
Link to deployed app on Heroku Check
Overall Nice work getting the optionals! Great work as usual. This is excellent.

@vertige
Copy link
Author

vertige commented Nov 6, 2017

Fixed the external link thing, totally didn't check for that!

I've been looking for ways to handle the placeholder text (ex disappear on focus, etc) but haven't found a way that works with css/html only and foundation yet. It annoys me, too, but apparently is the more appropriate option (so says Stack...) for the purposes where it's required. Since this isn't critical, I could make it go away... But it also slowed me down when going through my site... 5 calls per minute, ya know?

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