Skip to content

Instantly share code, notes, and snippets.

@ellytea

ellytea/code-review-checklist.md

Forked from thatPamIAm/code-review-checklist.md
Last active December 10, 2018 18:49
Show Gist options
  • Save ellytea/3c99f4aee42a65f244a87f5a871870b2 to your computer and use it in GitHub Desktop.
Save ellytea/3c99f4aee42a65f244a87f5a871870b2 to your computer and use it in GitHub Desktop.
Download ZIP
Code Review Checklist
Raw
code-review-checklist.md

Pair-to-Pair Code Review

PLease check:

  • Is all the code easily understood?
  • Is there any redundant or duplicate code?
  • Does the code follow the principle of single responsibility?
  • Any other feedback is welcomed.
  • https://github.com/ellytea/wheel-of-fortune

5 mins

  • Get together with your group and send a link of your team's GitHub Repo to the pair that is reviewing your project.
  • Look over this guide with your project partner and highlight the pieces where your team can use assistance/you would like your reviewers to take a closer look. Fork this gist and send the forked copy over to the pair reviewing your code.

45 mins

  • Follow the instructions to clone down the repo you are given and start reviewing the codebase. Take notes for written feedback.

5 mins

  • Come back together in your larger group. This time should be used to have an in-person discussion about the feedback. Send your formal written feedback as a gist/document to the pair and your instructors.

Review Guide

While reviewing the code, ask yourself the following questions:

CODEBASE

  • Does the code work? Does it perform its intended function, the logic is correct etc.
  • Is all the code easily understood?
  • Does it conform to Turing's Style Guide? These will usually cover location of braces, variable and function names, line length, indentations, formatting, and comments.
  • Is there any redundant or duplicate code?
  • Does the code follow the principle of single responsibility?
  • Can any global variables be replaced?
  • Is there any commented out code? Any console.logs?
  • Do the names used in the application convey intent?
  • Do tests exist, and are they comprehensive?
  • Do unit tests actually test that the code is performing the intended functionality?
  • Does the repo have a .gitignore file? Is there a .DS_Store file or node_modules directory committed that shouldn’t be?

README

  • Does the README follow the conventions we discussed in class (i.e. Abstract listed first, install instructions second, everything else listed after)
  • Is there a link to the DTR for the team available?
  • Is there a wireframe for the project available?
  • Is there anything that is confusing or hard to understand with what is written?
  • Does the README do a goob of showcasing this project? If you were a random human who happened to come across this project, would you explore it further based on the current state of the README?
  • What suggestions would you make for making the README better?

GITHUB/ISSUES/WORKFLOW

  • Are issues being used to keep track of tasks, enhancements and bugs?
  • Do the titles/descriptions of the issues provide all the relevant/necessary details for the issue? Would you be able to jump right in and assist?
  • Are other strategies being used to keep track of issues (labels, milestones, assignments)?
  • Is there effective communication happening in PRs outside of working school hours?
  • Do the PRS follow the flow that we discussed in class?
    • Summarize the changes that you made.
    • Give the reason WHY you made those changes
    • Ask for any insights

Feedback

You will be writing a formal piece of feedback (with your partner) to give to the team whose project you reviewed this morning. Use these questions as a guide; however, you are also free to give additional project feedback on anything that may have been overlooked with the questions listed above. Feedback is due by the end of the session. Please save your feedback as a file or gist and send it to your instructors and the pair. Remember to follow the feedback guidelines you went over as part of your professional development in Mod 1.

@trbachmann
Copy link

User Experience:

  • The design and layout looks great and is very engaging! Maybe consider adjusting the layout so it is on one page. Right now the user needs to scroll to see the bottom of the letter area.

scripts.js:

  • Is line 7, needed for the code to run correctly? At first pass is seems like it is instantiating one Player object, but then in Game.js the createPlayers method is instantiating three Player objects.
  • Nice use of setTimeout in the spinner function.

Game.js:

  • For testing purposes, maybe considering capturing the values of inputs (like line 11-13 on Game.js) to the scripts.js or the domUpdates.js files

Puzzle.js:

  • Nice use of reduce and unshift methods in combination with Math.random to return a randomized puzzleArray.

Wheel.js:

  • Nice work generating a random wheel element.
  • We noticed that there is only one wheel element being assigned from the whole data.wheel elements, but the project spec requests each Wheel to have six elements. That might be something you're already planning to refactor, but wanted to note just in case.

domUpdates.js:

  • Nice work taking into account if the user(s) does not enter a name in each player input field.
  • Great idea to use data attributes on your letters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment