Skip to content

Jacob Lyerla -Resubmission#272

Open
JacobLeonLyerla wants to merge 6 commits intobloominstituteoftechnology:masterfrom
JacobLeonLyerla:master
Open

Jacob Lyerla -Resubmission#272
JacobLeonLyerla wants to merge 6 commits intobloominstituteoftechnology:masterfrom
JacobLeonLyerla:master

Conversation

@JacobLeonLyerla
Copy link

No description provided.

First commit, set up my basic stuff and filled out answers.
finished up models and moving onto the end points
set up server to my routers, and used express.json
have tested everything in postman it seems to work, so that is good
forgot to push this
@ghost
Copy link

ghost commented Jul 23, 2018

Commit Guideline

I have generalized these as opposed to giving you personalized commit critiques. Surely you can discern for yourselves which of these recommendations you have or have not been following.

If you are already following all of them, good on ya. If you aren't, for your sake to be prepared in the professional world / be able to pinpoint where in code you need to look to see what changes you made more easily, please begin to do so.

  • Commit constantly! Don't go overboard and do it with every line of code, but any time you finish a component, write an essential function, etc. just throw a commit up there.
  • Make sure to use a title and body. If you are doing this from the command line, it will look like this: -m "Title" -m "Body".
  • Think of what the title should be similar to if you were writing a book. "part 1 done" doesn't tell you much unless you take the time to go find out what part 1 entailed. Instead "Function for Dynamic File Reading Completed" is pretty obvious what you did.
  • While body messages aren't always necessary, it is still usually a good idea to include them anyways. From the above example, an ideal message would be "filename.js --- used readFile and regex to parse the program correctly." Is that necessary? Not really. Later on, if you forgot where you were doing the dynamic file loading and you had that message, you wouldn't have to dig through the files to find where it was. That doesn't really apply to our tiny programs that have a few files, but if you have way more it can really help. Think back to the front-end project for instance.
  • In general, you should only commit working code. If you like to self-document what you did wrong to better your understanding, then a broken commit here and there isn't the end of the world. I would suggest though, that if you want to do this, title it something akin to "Unfinished Dynamic File Reading" or "Broken Dynamic File Reading".

Code Review

Good

  • Everything works great. MVP achieved.

Bad

  • I have some things to consider, but everything works.

Consider

  • Using a spell checker for written documents. I knew that you meant models and not modals, or brackets and not brackers, but good grammar / spelling is important in a professional.
  • In general, just take more time on the written portions. Question three for instance was correct, but you failed to mention express.
  • In general, also take time to make your code readable. Prettier or other extensions are crucial for me personally.
  • My OCD enjoys seeing uniformity in code, and I just think it looks better / more professional. For instance, I would stick to one of these but not both:
res.status(500).json({errorMessage: "The Budgets information could not be retrieved."})
res.status(500).json({ errorMessage: err });

Or some amalgam of the two such as:

res.status(500).json({errorMessage: "The Budgets information could not be retrieved.", error: err})
  • In the spirit of uniformity and spelling/grammar, you have both "categorys" and "categories", of which you should only have the latter.

Overall, solid Sprint Challenge.

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.

1 participant