Development process

Git

Git Flow

We use git flow to manage branches and deploys. In general, develop is the main working brach. When you start a new feature/bug/chore, you create a branch called feature/short-description (or bug/short-description, etc.) off of develop. Then make commits, and when done put up a PR. After getting a code review and an approval, merge to develop, which will kick off a deploy to staging.

When all features currently on staging have been accepted in Tracker (or you otherwise decide to deploy), make a release/YYYY-MM-DD branch from develop. Then you can make any final changes, and run yarn version:bump to update the version numbers of the public packages, and do a final PR. This will show you all the PRs included in the release, to be used for release notes. You should write up short descriptions of the new features in the PR, and post them in slack. Then make sure the release branch targets main instead of develop, and merge to deploy to prod. Finally, back-merge main into develop with git merge main --no-ff to keep the two branches synced.

If there is something that needs to be fixed in production without merging in everything from develop, then you can make a hotfix/short-description branch from main, make commits, and do a PR back to main. When merged this will update prod. Then you can git merge main --no-ff onto develop to bring in the changes to the working branch.

Commits

Commits should generally be atomic - small, related changes with a useful description. The point here is that when you later don't understand a piece of code, the commit message should help answer the question "why did someone make this change, and what does it accomplish?". Atomic commits are also great for allowing you to use git bisect to find where a bug got introduced, so you can revert or fix that change.

Not all commits will end up being small, and that's fine - but it's a good goal to strive for!

Merging in changes from develop

You want to regularly merge in changes from develop onto branches. When working by yourself on a branch, you can choose to rebase changes in from develop. This keeps a cleaner git history, at the expense of something requiring you to fix the same conflict multiple times, and the risk of rewriting history and blowing away a commit unexpectedly.

So, if you are working on a branch that others are also committing to or have branched off of, you should stick with merging in chagnes from develop instead. If you find youself in a situation with a complicated rebase, you can also fall back to merges - or just merge all the time if that works better for you.

Rebasing

Rebasing process:

git checkout develop
git pull
git checkout feature/your-branch # or `git checkout -` to go back to your last branch
git rebase develop
# Fix any merge conflicts
# `git add` changed files
git rebase --continue # continue to the next commit
git push --force-with-lease

If you run into a conflict in yarn.lock while rebasing, accept incoming changes and then re-run yarn bootstrap

Merging

Merging Process

git checkout develop
git pull
git checkout feature/your-branch # or `git checkout -` to go back to your last branch
git merge develop

Pull Requests

A good PR should follow the pull_request_template.md, and include a Tracker link, a short description of what the PR will accomplish or the bug it is fixing, a list of high-level changes (often these will correspond to your atomic commits), and a screenshot or GIF of the feature in action. The screenshot is particularly useful for the reviewer to quickly understand what the code is doing.

Once the PR is up, it's also nice to go through and do a review of your own, posting comments describing changes that may not be obvious, and also directing the reviewer's attention to parts of the code you're unsure of. The goal here is to try to answer possible review questions ahead of time, to speed up the review process.

Small PRs can be merged without a review, but later PRs should ideally have at least one Approval before merging. Whenever you put up a PR, you can post a link in the #develop slack channel to let other devs know it's ready to review. If you think someone in particular has context to review the code, feel free to call them out with an @mention in the #developers channel in Slack.

Reviews

When doing a code review, you have a few goals:

1. Knowledge sharing: understanding the code well enough to make changes in it if there's a new feature in the same part of the codebase that you pick up later. And If you don't understand what a piece of code is doing, then the PR is a good place to ask (and request either a comment explaining it) - otherwise you're going to be even more confused when you're looking at that code a month later!

2. DRY: You're looking for duplicated code in the PR that could be extracted into a shared function, either between parts of the PR, logic that already exists elsewhere in the codebase, or logic elsewhere in the codebase that could also take advantage of new functions written in the PR

3. Established code style and patterns: making sure that the new code fits into patterns and styles established in the codebase at large

4. Unknown invariants: Do you know something about how the codebase or libraries works that means there's some case where the new code will break? Things like "importing that file will increase the bundle size", "this will cause extra React renders because the object is re-created in every render loop", "this may throw an unhandled error".

5. Future refactors: Sometimes you may see something that looks like it could use a refactor, but may not be worth blocking the merge for. In that case still suggest it! The PR developer may agree that it's worth doing now. Otherwise, make a chore story for the refactor in the icebox so you don't forget about it.

6. Finally, for a complex PR, it's often a good idea to actually check out and run the code, to see if it works the way you expect it to

As you go through the review, you can add suggestions for fixes or improvements, and questions about things you don't understand. If everything looks good, you can approve the PR. If you see things worth fixing or changing, you can Request Changes. Even if your suggested changes are small or optional, still hit Request Changes, so the developer knows you have comments, and you know there may be more changes to review again.

When reviewing a PR, or requesting a second review, it's good practice to ping the developer/reviewer in slack, since github emails often get missed.

Environment Variables

We use a homegrown solution for managing environment variables. The majority of the environment variables used by our projects are specified in a single file, env.json, which lives primarily in cloud storage but is downloaded to packages/shared/env.json every time you pull code from the server. It can also be pulled at will by using yarn env:get.