Workflow

We are using the Github flow, which is mainly:

one master branch that is always production ready and in a deployable state 🛰️
one branch by feature or bug - a.k.a. feature branch
make a Pull Request (PR) on master for your feature / bug fix to be merged and go in production 🚀

No overhead

That's it!

We don't have staging, v2, develop, etc. branches. Just master and feature-XXX .

It tends to complexify workflow, merging, rebasing and such. Plus create stale code that is not pushed. All code not in production is dead code.

Creating a branch

Check out latest version on master, create your branch and code.

git checkout master
git pull
git checkout -b feature-name

Branching from an updated master branch minimizes the chances of conflicts when merging your code afterwards 👍️

Choose an explicit and straightforward branch named, based on our **commit convention **(see below):

doc/add-installation-readme
fix/login-error
feat/notification-settings

Ship it

Whenever possible we rely on continuous deployment.

Continuous deployment (CD) is a software engineering approach in which software functionalities are delivered frequently through automated deployments.
Source Wikipedia

That means: we code, we merge, we deploy. Instantly. Everything that is merged on the master branch is deployed in production right away.

Continuous deployment:

allows shorter feedback cycles
minimizes the delay between the introduction of a bug and the error happening in production, hence reducing the cost of a bug
minimizes the amount of code shipped (it's only 1 feature / bug fix), reducing the risk of failure or debugging overhead in case of a bug

Requirements

Continuous deployment requires the following to work:

code reviews - a peer review is the most effective code quality tool
an extensive test suite - to catch bugs before a user
a continuous integration pipeline - to run the test automatically
deploy previews - to check the output of the code in a production-like environment

Code reviews

Code reviews are part of our DNA and mandatory to ensure coding quality across a team and various projects.

Code reviews are more than just reading another's person code to check for typos of missing comas:

their author is writing code that will be read by someone else. It forces him (more or less consciously) to pay extra attention to what he's writing.
it shares the knowledge of a project among multiple developers, limiting the bus factor
it removes ownership of code by transferring some responsibility of the code to the reviewer

What code reviews should be about; everything that can be only spotted by a human 👩🏻

looking for introduced bugs and edge cases
discussing about design patterns and implementation
actively looking for _what's missing _(e.g. changes that should have been done at another place as well)
properly naming things (because you know, naming is hard)

What code reviews shouldn't be about; what a robot can take care of 🤖

discussing style (parenthesis, comas, that sort of stuff... 🙈). This is the linter's job
bike shedding 🚲️🏚️(i.e. focus on details)

Preparing code for review

Your code will be reviewed by a peer, he's taking time and energy to stop working on his things and "help" you on yours. You want to make that as smooth and easy as possible 🏄️

Before asking for a review:

check your commits in your IDE or in Github (make sure you didn't accidentally commit a file, or left a typo)
make sure all tests pass
try avoiding making and unmaking things in the same PR; it's harder to grasp for the reviewer
comply to clean commits and clean code philosophy

Clean commits

Commits are snapshots through history, they allow traveling through time 🚘, finding where a bug was introduced, understand the evolution of a feature or track a Eureka moment when finding that tricky bug 💡

We make atomic commits ⚛️

Atomic commits are:

one irreducible feature, fix, or improvement
easier to review - be nice with your peer developer 💆‍♀️
are easier to rollback - ever done a git bisect or try debugging a lockfile with multiple new package installs? 🤦‍♂️

Our commit messages and philosophy are inspired by a number of projects, such as Conventional Commits or Angular

Commit types

One commit is of one type only (otherwise can't be atomic). This list is taken from the style guide

feat (feature)
fix (bug fix)
doc (documentation)
style (formatting, missing semi colons, …)
refactor
test (when adding missing tests)
chore (maintain)

Commit message

You never want to debug or review a PR full of fix1, working now, really working now commit messages. Even if you make more expressive messages, remember they must be intelligible for another developer... and even harder, the future you 👨‍🚀

We use a convention so it's easy for everybody to immediately grasp what a commit is about:

<type>(<scope>): <subject>
<BLANK LINE>
<body>
<BLANK LINE>

Example:

chore(eslint): allow unescaped `"` and `'`

- because they exist in french 🇫🇷🤷‍
- for the sake of simplicity
- @link https://github.com/yannickcr/eslint-plugin-react/blob/6bb160459383a2eeec5d65e3de07e37e997b5f1a/docs/rules/no-unescaped-entities.md

Some guidelines:

keep the subject short and straightforward
use active and present tense; e.g. add style to Button component)
add links to documentation and/or issues (especially if you had a hard time finding it)

Clean code

Rules of thumb for a cleaner codebase and happy relationship with your peers:

NEVER commit commented code. We use git for that; keeping track of the past. Delete it, make a clear and understandable commit message, and forget it ☕️
Delete dead code. All code that is not used is stale, clutters the codebase, adds overhead to the developer and prevents moving fast. Delete it 🗑️
Use comments wisely: explain the why, not the how (that's what the code should do). Explain uncommon solutions.

Keep in mind that stale comments are worse than no comment, and keeping comments up to date is hard
Name your variables with care. Variable names are made for humans, they convey meaning
Code with intent: prefer an expressive coding structure over a smarty one-liner, that happens to work but using a clumsy side-effect of what it's originally meant for... 🤔You'll understand when you see it

the way you code should express what you are trying to do

Code style

Debatting over codestyle, such as "should we put a trailing coma in arrays", "should we add semis at the end of a Javascript statement" brings no value. At all.

We discuss these topics once and for all amongst the team and then enforce code style through a _linter _ and "beautifier" libraries. A good example is the combo eslint / prettier

eslint: lints the code for static errors, enforces specific rules such as styling conventions
prettier: automatically formats the code according to shared rules with eslint

These rules are checked and enforced by our Continous Integration pipeline.

You can't merge code that doesn't pass style and linting! 👮

We recommend setting up these tools in your IDE so you instantly see errors, and it formats your code automatically for you 🧙. For VS Code for instance:

Git project settings

Protect the master branch

The master branch is production. It is valuable, everything that is in master is considered live ⚡️. It shouldn't be easily breakable and it should be protected.

^{Go into Repository settings > Branches to activate branch protection rules}

Current branch protection rules are:

force pushing: you can't force push to master; this would risk destroying the whole history of commits, and the work of your colleagues (destroying the parent commit)

NB: everything that is on master is unchangeable, it's now part of the project forever. Don't change it.
code reviews: require a pull request review by 1 peer (whoever has writing rights to the repository). Doesn't matter if it's a junior or a manager; just needs review
continuous integration: build, tests, linting, etc. everything that is checked by a robot to ensure code quality

^{Simple protection rules}

We could add additional layer of protections such as requiring the upstream to be up to date, or dismissing stale PR approvals, but most of the time simple is good. If you want to add additional checks here are some recommended ones:

requiring the upstream to be up to date: makes sure git doesn't accidentaly merge your branch wrong (also this happen very rarely)
dismissing stale PR approvals: this is great in theory (can't change something after a PR approval, otherwise approval is useless...) however, it adds a lot of noise and complexity to the code review process. Can be replaced by trust 🤟 in coworkers
code coverage checks: tests are good. While code coverage isn't necessarily the perfect solution, it does help writing tests. You can add diff coverage check (i.e. the code you just added is covered by tests) and total coverage check (i.e. total coverage of your repository).
require linear history: forces developer to rebase their PR on the upstream rather than merging the upstream into their branch. Enforces history continuity and reduces merge-fail 🦶🔫

^{Advanced protection rules. Beware not to clutter your dev workflow though.}

Add security notifications

Every project has dependencies and these dependencies can have security issues. Github has a feature to automatically check vulnerable dependencies 👹, you need to enable it.

^{Security analysis settings}

nicolasrouanne/GIT_MANIFESTO.md