Skip to content

Lab 2: Collected Code Review Comments

Jump to bottom
Julia Ogris edited this page Jun 4, 2019 · 1 revision

Thank you for submitting your PR!

A couple high level comments (see below for details):

  • Don't label your PR that is failing CI build as Ready for review
  • Fix your PR description
  • Fix commit messages
  • Limit your number of commit messages to one or two
  • Don't forget to git pull origin FEATURE-BRANCH after Update branch
  • Don't commit commented out code
  • Don't use global variables that can be scoped local to functions
  • Don't commit binaries
  • Don't capitalise error strings (see official recommendations)
  • Use table driven tests to avoid repetition (slide sample, Dave Cheney post)

PR description

Please make sure you use proper Markdown (cheat sheet) in your PR description and fill it in accordingly:

  • You need to create a new issue that you reference in your PR description:
    Fixes #<NEW-ISSUE-NUMBER> means it will be automatically closed when the PR is merged.
  • Please also fill in Changes proposed in this PR:

This could look something like:

Fixes #<NEW-ISSUE-NUMBER>

Review of colleague's PR #<PR-NUMBER>

#### Changes proposed in this PR:
- Implement Lab2 Bubble sort
- Test Lab2

Commit messages

https://chris.beams.io/posts/git-commit/

Use git rebase -i COMMIT-HASH to rework your commits

  • Separate subject from body with a blank line
  • Limit the subject line to 60 characters
  • Use the imperative mood in the subject line
  • Do not end the subject line with a period
  • Wrap the body at 80 characters
  • Use the body to explain what and why vs. how

E.g. Change

Lab 2 completed.
Built bubble sort code to binary

to

Complete Lab 2
Build bubble sort code to binary

Limit the number of commits to one or two

You can squash several existing commits into one with git rebase -i COMMIT-HASH and git push -f. For the future, if you have fixup commits use:

git commit --fixup COMMIT-HASH
git rebase -i --autosquash PREVIOUS-COMMIT-HASH
git push -f

Don't forget to pull from origin

Don't forget to git pull origin FEATURE-BRANCH after you have clicked Update branch on github.com. Clicking Update branch merges master into your feature branch FEATURE-BRANCH on the remote. Alternatively to clicking the button you can run locally:

git checkout master
git pull upstream master # or just `git pull` if `master` is set to track `upstream/master`
git checkout FEATURE-BRANCH
git rebase master
git push -f
Clone this wiki locally