Git & Github

Version control is a project's best source of documentation when done correctly. When trying to understand code it's extremely useful to use git blame to find both the PR and the issue associated with that change.

PRs should be small and focused. Each commit should solve a single problem and be covered by a test that exemplifies that particular feature or fix.

All PRs must be reviewed by a teammate before they are eligle to be merged into the master branch. Large PRs are difficult to review. Be sure to break large PRs into smaller ones so that they can be reviewed quickly and deployed to production.

• Never commit passwords, access tokens, or other credentials into version control. If you think you absolutely have to, ask first. If you do this by accident, tell someone immediately.
• Each commit should be as small and as simple as possible.
• Write the commit message in the imperative mood.
• The project must be operational and have all tests passing after every commit.
• Use Conventional Commits
  • See the "Commit Types" section below
  • Valid types are chore:, docs:, style:, refactor:, perf:, and test:
• Do not mix feature changes (added functionality) with fixes (restored functionality), refactors (no change in functionality), or style changes (only whitespace or other cosmetic changes).
• Before a PR is ready for review, make sure that it is a single commit. If the combined commit is too large or disparate, consider multiple PRs.
• The exception to the above single commit rule is when a PR introduces new packages. Create one extra commit in the same PR for each new package your PR needs.
• Do not modify a project's .gitignore to add files related to your editor or environment. Use your own global .gitignore for that instead.
• Be sure that your PRs have descriptive titles that explain what has been changed. Typically the commit message is sufficient. "Fixes #66" is not.
• When submitting a PR with UI or visual changes, please add before and after screenshots to the PR. This makes it easy for the reviewer to quickly see what has been done.
• PRs should be rebased and merged. There should be no additional merge commit.

Parent branches

With larger features, it can be tempting to create a parent branch, allowing for us to work on a feature in a "rough draft" state over a period of time. The challenge there is that keeping the parent branch up-to-date with master can be tedious, resulting in work not being merged into the parent branch. To get around this, we have a number options available, depending on the work being done.

To get around the need for a parent branch, put the new work behind a feature flag. This allows for making small incremental changes to a feature, hiding the fact that it is in a "rough draft" state from the users. This can also work if we are updating an existing feature that requires several days of work: create a new component by copying the old component. Once the new component is ready, remove the flag and delete the old component. If you are working on an entirely new view, you may not need the feature flag. The user will only see the view when we link to it from existing UI. Until that happens, we can merge work into master without having to worry about making sure the feature is "complete".

Workflow

We use Zenhub to manage our workflow. Each task is represented by an issue. Be sure to connect any PR you are working on to the appropriate issue. Do this via the Zenhub interface and by adding Closes #X where X is the issue number to the PR description in Github.

As tasks move from Backlog to In Progress to Needs Review to Needs QA to Ready to Deploy and finally to Closed.

• Backlog: This is where you will choose from issues to work on. Once you have selected one, move it to In Progress.
• In Progress: While you are working on an issue, it should stay in this column. Once you are finished and satisfied with your work, move it to Needs Review.
• Needs Review: Each day you should be looking at this column for PRs to review. This column will list all issues that are complete and are waiting on review before they can be ready for deploy. When you review an item, make sure that it follows all of our coding principles and will not cause problems when we deploy it to production. Once a PR has been reviewed, either move it back to In Progress or forward to Needs QA or Ready to Deploy. Do not allow items to sit in Needs Review.
• Needs QA: If the change affects the front-end and can be tested with a deploy preview it should be tested by QA. After successful testing it should be moved to Ready to Deploy, but if it is unsatisfactory, it should be moved back to In Progress.
• Ready To Deploy: After an issue is finished and has been reviewed and approved by a teammate, it will be moved to this column. After it has been deployed, it will be moved to the Closed column.

Slack & Pull Requests

We have a dedicated Slack channel for posting PRs: #engineering-prs. Every PR should be posted in this channel, allowing for others to know that you have work that needs review. Communicating updates about the PR will happen in a thread attached to the original Slack message for the PR.

The process for posting a PR in Slack is:

• Post your PR in #engineering-prs
• If you are the developer reviewing the PR, start a thread, saying: @<developer_who_made_PR> reviewing
• If the PR needs to be pushed back, the developer reviewing responds in the thread with: pushing back to In Progress @<developer_who_made_PR>.
• If all comments have been addressed, the developer responds in the thread with: @<developer_who_made_the_comment> all comments have been addressed. Pushing back to Needs Review
• If you have approved the PR, respond in the thread with: @<developer_who_made_PR> approved

Please note that all discussion about the PR should stay in GitHub. Threads are not intended a place to discuss the code itself. Slack is only used to provide more immediate feedback.

Commit Types

Commit types (e.g. feat, fix, refactor, style) are important because they are a signal to the reviewer for what they should be looking for and how much scrutiny the review needs. Reference from Angular:

• feat: A new feature
• fix: A bug fix
• refactor: A code change that neither fixes a bug nor adds a feature
• style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
• test: Adding missing tests or correcting existing tests
• docs: Documentation only changes
• perf: A code change that improves performance
• build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
• ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)