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

Matt Ray submitting 'custom-api' homework #1

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

Matt Ray submitting 'custom-api' homework #1

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 6, 2017

TangJames pushed a commit to TangJames/custom-api that referenced this pull request Oct 6, 2017
so-chow added a commit to so-chow/custom-api that referenced this pull request Oct 11, 2017
@jhnsnc
Copy link
Contributor

jhnsnc commented Oct 16, 2017

Your models are clearly organized--definitely a benefit of starting with what we came up with in class for the "food tracker" API. It doesn't look like you got to actually use most of the models; perhaps it might have made sense to simplify the models until you had coded more of the endpoints?

Overall, there's a lot of cleanup to do on this repo. I see a lot of completely unused code (e.g. /controllers/routes.js, bottom half of /routes/routes.js, views/*). It's fine to be a bit messy while you're developing and trying to figure out what to do, but make sure you go back and clean things up once you get it working. Can't call it quits right when you get it working--getting it working is just step 1.

I liked seeing the clean, consistent formatting in routes/helpers/routes_helpers.js. Makes sense that it would be cleanest since you were probably spending most of your coding time in that file.

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