Skip to content

Instantly share code, notes, and snippets.

@nicolasrouanne
Last active May 6, 2020 11:37
Show Gist options
  • Save nicolasrouanne/ef61dce4974a0398ac882939a2e29b78 to your computer and use it in GitHub Desktop.
Save nicolasrouanne/ef61dce4974a0398ac882939a2e29b78 to your computer and use it in GitHub Desktop.
Git Manifesto

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.

branch-protection-rules
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
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
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
Security analysis settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment