Versions Compared

Version Old Version 17 New Version 18
Changes made by

Walter Moar

Walter Moar

Saved on

Key

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

...

  1. Unless you’re the person most familiar with the area of the codebase that you are changing, first discuss the changes with that person

Expand
titleWhat does Why is this meanimportant?

There will always be improvements underway in the CHEFS code, and any new work must align with those improvements. The end goal of these improvements is code that is:

  • necessary (YAGNI - don’t maintain, document, test, etc, code that we don’t need)

  • correct (it should work!)

  • simple (“as simple as possible but no simpler”)

  • documented

  • easy to read

  • easy to understand

  • easy to modify

  • easy to fully test

  • consistent (to reduce cognitive load)

Information that everyone should be familiar with:

...

Expand
titleWhy is this important?

The goal for all developers is to create Pull Requests that are easy and fast to review. If a task contains a large amount of work, break it down into smaller stand-alone tasks. Jira subtasks are one way of recording this, and do allow the task subtask to go through the various Jira swimlanes.

...

Expand
titleEverything you ever wanted to know about the unit tests

Test coverage reports appear in:

  • Frontend: app/frontend/coverage/index.html

  • Backend: app/coverage/lcov-report/index.html

Refer to the backend unit test documentation for details on:

  • test writing strategy

  • running tests on the command line

  • running with and without the coverage report

  • running single specs or single tests

  1. Publish your branch to your fork, for example by using the “Publish” button in VS Code

  2. Create Start to create a Pull Request for your branch, for example by using “New pull request” in the GitHub web site

  3. Do a self review (diff)

  4. (somewhere above: limit size of change)

  5. Enter the “title” for your Pull Request in the format feat: FORMS-1234 new map component for dropping a pin

Expand
titleWhy this format is mandatory

We use the format type: FORMS-NNNN description for our Pull Request titles so that:

  • the type of change is obvious from the name of the Pull Request

  • it is easy to find all Pull Requests for a given type like feat or test

  • it is easy to find the Pull Request for a given FORMS-NNNN Jira task

  • the description makes it easy to scan Pull Requests to find a change that happened

  1. Enter the GitHub “description” for your Pull Request using the template provided. The template contains comments to help make the process easier

  2. Before clicking the create button, read through all the file diffs

Expand
titleWhy this is recommended

This is a chance to do a self-review of your changes before creating the PR.

  • are the changes too big (too many files, too many lines) to be easily and quickly reviewed?

  • is the Pull Request doing too many different things? Can it be broken down into smaller PRs?

  • is every changed line necessary? Was anything accidentally committed?

  • are there tests to cover the code changes / additions?

  • are there documentation updates for the code changes / additions?

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

  2. Wait for the “Tests” GitHub Action to automatically run 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 reviewers

  5. Talk to reviewers

  6. Keep up to date

  7. Deploy Action

  8. merge and message

  9. Push Action

  10. Undeploy Action

  11. Code Climate / etc?

  12. Jira: moving tickets, rewrite description

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

...