Versions Compared

Version Old Version 18 New Version 19
Changes made by

Walter Moar

Walter Moar

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  • Code changes that follow the design and style of the existing code

  • Code changes that do not decrease test coverage or maintainability

  • Commit messages and Pull Requests that are in a consistent format

  • Pull Requests that are easy and fast to review: avoid the review bottleneck

After a developer is assigned a Jira task, they begin work.

...

  1. Click the “Draft pull request” button to create the Pull Request

  2. Wait for the automatically-run “Tests” GitHub Action to automatically run complete successfully

Expand
titleWhat tests are run?

The “Tests” GitHub Action runs:

  • GitHub CodeQL tests to look for security issues

  • Backend Jest unit tests, including test coverage saved as an artifact

  • Frontend Jest unit tests, including test coverage saved as an artifact

Note that the test coverage is not uploaded to Code Climate, as Actions run from Pull Requests do not have access to the GitHub Secrets needed to authenticate with Code Climate.

  1. Run the deploy Action

  2. test

  3. Mark as ready for review

  4. Add reviewersUsing your Pull Request number, run the “PR Deploy” Action

  5. Test your changes in the deployed PR environment

  6. Do a self-review of your Pull Request

Expand
titleWhy is a self-review needed?

Pull Requests should be created so that can be easily and quickly reviewed. By reading through every line of your code changes (because the reviewer should also be reading every line) you check that the Pull Request will make sense to your reviewers. It might help to add a GitHub comment to your code changes to explain them better. It might help to split your Pull Request into smaller pieces if it is too large: small Pull Requests that take ten minutes to review will get a more thorough review than Pull Requests that take hours or days.

  1. Click the “Ready to review” button to take your Pull Request out of “draft”

  2. If necessary, add reviewers

Expand
titleIf necessary?!

Ideally every developer would review and understand every change in every Pull Request, but that’s not practical. There are two ways of looking at Pull Requests:

  1. Those use existing concepts / designs vs those that introduce new concepts / designs

  2. Those that are simple changes vs. those that are complex changes

We can use the Ship / Show / Ask approach to changes:

  • Existing concepts / designs + simple changes: Ship it (example: dependency update, typo fix, small bug). Show or Ask if you are uncertain or otherwise feel it’s necessary

  • Existing concepts / designs + complex changes: Show / tell about the changes but doesn’t necessarily need a review (example: new feature that’s very similar to existing features). Ask if you are uncertain or otherwise feel it’s necessary

  • New concepts / designs + simple changes: Ask for a review (example: small bugfix that needs a big refactoring)

  • New concepts / designs + complex changes: Ask for a review (example: new feature that’s different from anything we have)

  1. Talk to reviewers

  2. Keep up to date

  3. Deploy Action

  4. merge and message

  5. Push Action

  6. Undeploy Action

  7. Code Climate / etc?

  8. Jira: moving tickets, rewrite description

What is the intention of a PR review? Ship / Show / Ask. nitpicks. style guide. choose your battles

...