QAing and deploying code

Reviewing code is a task that is generally handled by both the developer themselves and a reviewer(s). Broadly speaking the developer will take things as far as they can on their own with the goal of minimizing the reviewers time and energy. On the flip side, the reviewer(s) will provide a second set of eyes to validate acceptance tests and code quality.

The specific action items assigned to a developer and reviewer can differ from project to project and are generally enumerated in a given project's pull request template. For example, here is the template for this repo. You can read more about how that works above.

That said here are the two general flows you'd take depending on your role.

Developer Flow

1. Housekeeping

On a high level this means making sure the PR is as ready to go as possible. It could include making sure your code has pulled in the latest from master, that any relevant doc changes have been reflected or that you've written an acceptance test.

If a reviewer has to remind a developer they've missed basic things on their checklist it vastly impacts team flow.

2. Automated Tests

The developer is also responsible for making sure all automated status checks contained in the PR have passed before they request a review. Generally, these tests will handle things like:

  • Code linting and code standards enforcement
  • Unit and functional tests
  • Code analysis and review

If the light is green then the trap is clean!.

PR status checks

Reviewer Flow

1. Acceptance Tests

Each pull request will automatically spin up a separate QA environment on Platform.sh. This enables a manual acceptance testing flow by stepping through a user journey on the QA environment and verifying it does what it is supposed to do.

It also allows us to compare user journeys between the QA environment and some control environment (usually dev or production) to test bug fixes.

Every pull request template will generally have an example acceptance test for the developer to flesh out. This means that the reviewer is not responsible for writing this test, only verifying it does what it says it does.

2. Code Review

The final responsibility of the reviewer is to ensure the code is good enough to be merged. Generally, this means

  • Looking out for any low hanging logic errors or cheap improvements
  • Ensuring the code fits into the projects architectural plan
  • Ensuring the code is not defining a new way to do something we already have a pattern for
  • Reviewing relevant code analysis feedback eg a Code Climate Report

If you are unfamiliar with GitHub's pull request review system then check out these docs.

3. Client review

@TODO: Dustin and MM?

4. Deploying Code

Once the PR template checklist is complete and both the client and reviewer are feeling good about things you may press the button