-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Hey Calvin!
I am taking a look at your project and I wanted to send you my thoughts. You clearly knocked it out of the park, so you're in really great shape. The UI isn't the greatest, it must be said. I know you aren't necessarily a UI guy (and this isn't a UI class, so I won't hold it against you), but it would be my primary (and perhaps only) critique against the app.
You certainly went above and beyond on your readme - in fact, potentially too much! You don't need to go into so much detail about each step. You can assume that the user either knows how to install and startup mongo, or that they will consult the mongo documentation if they have problems with it. Your instructions should really just cover the absolutely minimum steps required (install node and mongo, clone the repository, run npm install, set configs, run app with x). You can always have more extensive documentation further down in your readme or in the repository's wiki area.
Your command-line options are great, but in my opinion it would be easier to have the mongo options in a config file. There are really no security problems hard-coding development options, and then you can have a separate config which uses environment variables to get production values (if you haven't discovered it yet, look into the 'NODE_ENV' environment variable). The tricky thing is that most production environments will just start your server using "npm start" so you don't have much control over those command-line options on your production server.
Along those lines, my local mongodb has no username or password. I had to dig into the code before I discovered how to disable that requirement. I did see your note about the 'mongoUsesAuth' config, but only after I already found it in the code. This is a good example that your startup instructions are a bit too verbose.
I definitely appreciate the documentation in your notes directory. You could consider posting those to your github repository in the "wiki" section, if just to have more formatting control. It inspired me to start thinking about how to automate API documentation, and I came across this npm package: https://www.npmjs.com/package/express-mongoose-docs. Yet another thing to investigate!
You have commented each of your Angular files pretty well, but the thing about commenting is that you can never have too many (in my opinion). You could include a description of each method within your controllers in addition to describing the controllers themselves.
I don't need to tell you that you're on solid ground here, since you already got a job. I hope you enjoyed working on this project, and I'll be on Slack (both on CodeLouisville and Louisville IO) if you have any questions about anything.