Code Review

Page last edited 3,688 days ago
From Artisan
Jump to navigation Jump to search

See the code with some (Github) comments on Github here (requires collaboration rights).

Notes from our code review (27 Mar 2014): The Item Interface

  • who had a chance to look at this? Mostly everybody
  • some style stuff—camel case, getters/setters
  • length of an item:
    • what specifically does this refer to?
    • more than one dimension?
    • should not have a SET method
    • should have a get method?
    • is there any benefit to leaving it in?
    • comment out and leave for later
    • collisions vs. ray tracing – better, Unity already equipped to do this
  • Item subparts – not really necessary for combat team, but crucial for item creation
  • Item template with baseItem—infinite recursion loop?
    • manage a private variable independently
    • build an item from the top up or from the bottom down (recursive construct())
  • overlayMaterials() – yellow feature
  • mass computed based on materials
    • Center of Mass concerns with Unity assuming a mesh is of uniform density
    • components of the Material class

Basic Workflow - Pull Requests[edit | edit source]

Our team has decided to use the Github 'Shared Repository Model' centered around pull requests, which is explained by Github here. The process is as follows:

  • Clone or update from the master branch
  • Do all coding/work on a new local branch
  • Commit changes with a specific description of your work to a new remote branch with the same name as your local branch (or commit to the existing remote branch with the same name if further changes need to be made after the code is reviewed)
  • Open a pull request of your code compared with the master branch and have someone else review it. Note that we never merge our own code into the master branch.
    • Note: To notify a specific user of your pull request, type @<username> in the commit message and it will notify them of your pull request.
  • The reviewer reviews the code (with or without the coder) and comments on any changes that need to be made. If the coder needs to make any changes to his code before it is merged into master, he commits those changes on the same branch he has been working on and has the reviewer look at all of the new code.
  • After all changes have been made to the coder's and reviewer's satisfaction, the reviewer merges the branch into master (manually if necessary), closes the pull request, and deletes the remote branch just reviewed.