Versions Compared

Version Old Version 21 New Version 22
Changes made by

Walter Moar

Walter Moar

Saved on

Key

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

...

We follow the development process below to have:

  • Code changes that follow maintain 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

...

  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
titleWhy is this extremely important?

This is extremely important because you don’t want to spend a week on a Pull Request only to find out that it has to be completely redone because of its core design.

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 allow the subtask to go through the various Jira swimlanesKanban stages.

  1. Using the type from Conventional Commits, decide what the primary type of work is, such as feat for a new feature

...

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

Please make the description meaningful:

  • BAD: feat: FORMS-1234 add feature

  • GOOD: feat: FORMS-1234 add map component

  • BAD: fix: FORMS-2345 fix bug

  • GOOD: fix: FORMS-2345 fix layout bug with Data Grid

  • BAD: test: FORMS-3456 add tests

  • GOOD: test: FORMS-3456 finish test coverage for email service

If your changes won’t fit in the description, your Pull Request is probably doing too many things.

  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?

...

Expand
titleHow do I know when the changes are deployed?

There are various ways to tell this:

  • Watching the Action you will see the Deploy step complete and display the URL of the deployment

  • Unless you have the notifications disabled, you should receive an email

  • The Action will add a comment to the PR that contains the URL of the deploymentPull Request

  1. Test your changes in the deployed PR Pull Request instance

  2. Do a self-review of your Pull Request

...

Expand
titleIs this always needed?

This step really depends on the reviewer. Some people notice that they have been asked to do a review, but many do not. It doesn’t hurt to post in the team channel on Discord that something is ready for review and “mention” the reviewers.

If your change is complex or if your team members are less familiar with that part of the code, it might not hurt to set up a meeting and explain the changes

  1. In Jira move the task to PULL REQUEST (PR)

  2. Keep your branch up to date with main and re-run the “PR Deploy” Action

Expand
titleHow often should I update?

It depends! It’s good to always be up to date because the testing and review is more meaningful if the base branch is up to date. However, it can also waste a lot of time if one person is putting in multiple small changes and everyone is trying to keep up. Find a balance between having a current base and spending time keeping it current.

...

You can use the GitHub web site to update your branch, but note that this leaves the branch in your local environment out of sync. This might be OK if the update is for completely different parts of the code, but if there are conflicts then its better to update locally and push to your branch.

Expand
titleIs this necessaryDo I have to re-run the Action?

If you or others are testing the deployed code, but the code is not up to date with main, then bugs could sneak through. When you look at your PR there will be comments in the timeline for commits, and comments from the PR Deploy Action. If there are commits after the last PR Deploy, then it should be deployed again. Retesting might be wanted if there were merge conflicts or other contention between the new main and the PR code.

  1. Wait for approval (if needed) and merge your changes with the properly formatted commit message like:

Code Block
feat: FORMS-1234 new map component for dropping a pin
 
Added a new map component that allows the user to drop a "pin" on the map,
and the location is saved as both Lat/Lon or UTM coordinates.
Expand
titleWhy is this format mandatory?

This format must be followed as we eventually want to introduce automated changelogs and versioning.

  1. Monitor the Push Action

Expand
titleWhat does this Action do?

The Push Action runs on merge, and:

  1. Build the applicationDeploys the application

  2. Does a no-outage deployment to dev

  3. Waits for gatekeeper approval to deploy to test

  4. Deploys the application Does a no-outage deployment to test

  5. Waits for gatekeeper approval to deploy to prod

  6. Deploys the application Does a no-outage deployment to prod

Once you have merged your changes, ensure that they are deployed through all environments. Do not leave a deployment “hanging” and deployed to only the lower environments, as the next merge will probably cause it to go all the way to prod. If the change is tested and approved, it must should go to prod.

  1. Run the PR Undeploy Action with your Pull Request number

Expand
titleCan this be automated?

Maybe? Merge can only be done by people with write access to the repo, so in theory on merge we would have access to the GitHub secrets and could undeploy automatically if we can figure out the PR number.

  1. Check Code Climate

Expand
titleWhat's Code Climate?

We use Code Climate to monitor maintainability and code coverage. However, it will probably be replaced by SonarCloud in the near future. But it doesn’t hurt to check that your changes didn’t decrease our maintainability or code coverage.

  1. Move Jira task to DONE

  2. Celebrate 🎉