Adhering to a common set of developer norms is a vital factor when a team of people is trying to produce high quality, timely, business critical software.
I'm regularly asked to work on codebases that do not conform to any known measure of coding standards.
Issues include, but are not limited to:
- no linting
- no, or few, or inadequate levels of automated testing
- no common README format
- out of date or simply innacurate README docs
- no common CONTRIBUTING standards
- no standards for PR, branch, and commit message naming
- too many branches, making it impossible for a new developer to know where to start
- poor choice of repository or project names
- repositories scattered across multiple accounts
- lack of enforcement of CI/CD use
Coding standards are, by their very nature, opinionated, and the following represents my own opinion, based on decades of professional software development, of what our common coding standards ought to be. These standards are mostly independent of programming language (I've included some language specific recommendations as well however) and present a 'best-practice' approach to developing high quality software.
-
For any codebase there will be a repository, known as the 'upstream' repository.
The benefits of enforcing this are as follows
- When a new developer joins the project it's easy for them to see immediately which branch contains the latest work in progress and which branch contains the latest release code.
- For public facing repositories it presents a clean and consistent public face for our codebase, showing us in the best possible light.
- Each developer can use their own fork to do whatever work they need to, and the onus is on them to keep their own repositories tidy.
-
The Upstream Repository will only have two branches,
develop, containing the latest working code, andmastercontaining the current release. -
The default branch will be
develop -
The
developandmasterbranches are locked down such that the only way code can be contributed to them is via a peer-reviewed pull-request. -
Over and above the default labels provided by GitHub add
documentationandfeaturelabels. -
Assign teams
adminswith admin rights, anddeveloperswith write access to the repo.
For work in progress developers will use the forked-git-flow process, namely they will
-
Fork the
upstreamrepository into their own personal accounts, clone the fork to your local machine, and set up theupstreamas a remote.git remote add upstream [email protected]:<repo>.git git remote set-url --push upstream no_push -
Create a feature branch. (I use the
git-flowcommand-line tools to do this, see below for discussion of branch, PR and commit message naming)git flow feature start ...Developers should not, as a general rule be working in other developer's branches, but if it's really needed they can by setting up another
git remote -
do work
-
ensure work is accompanied by tests
-
ensure test coverage is not decreased by the work
-
ensure work conforms to common linting standards (For Javascript projects I use eslint and prettier for this, but tools vary from language to language)
-
ensure work is consistent with current documentation
-
when work is done, publish the branch to your personal fork (
git flow feature publish ...) -
using GitHub to create a Pull Request against the upstream
developbranch (see below for PR naming scheme) -
assign reviewers, appropriate labels, and assign the PR to yourself
-
wait for CI/CD to run and give a green light
-
notify devs on slack that the PR is awaiting review
-
respond to any review comments / make changes as appropriate. (developer can simply
git pushtheir alts to update the same PR) -
when all changes / PR is approved, merge the PR and delete the branch
-
git checkout developthengit pull upstream develop, thengit pushto ensure your fork is in sync with the upstream.
We use a mix of TravisCI and CircleCI for our CI/CD needs. Whichever we use (each have their own pros and cons but are essentially similar, so this advice is agnostic) the CI/CD must be configured to
- run linter
- run unit tests
- run integration tests
- deploy to staging (in the case of a run against
develop), or production (in the case of a run againstmaster) - Notify Slack / Jira
Features must be named per the following pattern #{issue number}/{some_descriptive-text} — so for example, if you are working on issue ABC-1 with the title "do the thing", call your feature ABC-1/do_the-thing. Obviously use your common sense to avoid making the feature names too long.
Note this will creating a feature via git flow will create a branch called feature/{issue number}/{some_descriptive-text}.
When committing something use the -m flag to add a short commit message of the format {issue number} summary of what you changed. So for example if you are working on issue ABC-1 and you added a method to the aardvark_controller you might use the following commit message "ABC-1 added anteater method to aardvark controller"
Commit messages ought to be in the past tense.
In general try to group file changes wherever appropriate, so if your controller change also involved updating something in a helper file, the one commit message can happily encompas the changes to both files. The message ought to reflect the main aim of the change.
Pull requests must be named as follows [issue type, issue number] high level description of change. The following Issue Types are recognised
Bug Fix- the change fixes a bugFeature- the change adds a new feature (the usual issue type)Documentation— The change is a documentation only changeOptimisation- The change is an optimisation of the code base without any functional changes
If your change does not fit any of these categories, use Feature. Likewise if your change is not tied to an issue number you may use n/a instead.
So to use the above example your Pull Request would be named [Feature, ABC-1] added anteater to aardvark
Every project needs a file README.md written in Github Flavoured Markdown, with sections as follows
-
A short overview of the project
-
Current CI/CD status for both
developandmasterbranches (Both Circle and Travis provide badges for this) -
Prerequisites and expectied prior knowlede including links to associated repositories
-
Useage instructions
-
Development instructions
- How to build,
- How to test
- Any specific environment variables needed
-
Deployment instructions
-
Contributing nodes (ususally just an instructon to see CONTRIBUTING.md)
For Javascript projects I use eslint and prettier to enforce a common coding style, configured via the following base .eslint.json file. This can be enhanced with React specific plugins as required, but overall stick to the defaults but with enforced single quotes and no trailing semi-colons.
{
"extends": [
"standard",
"prettier",
"prettier/standard"
],
"plugins": [
"prettier",
"standard",
"mocha"
],
"parserOptions": {
"sourceType": "module"
},
"env": {
"es6": true,
"node": true,
"mocha": true
},
"rules": {
"prettier/prettier": ["error", { "singleQuote": true, "semi": false }]
}
}
For solidity projects I use solhint with the following configuration
{
"extends": "default",
"rules": {
"compiler-fixed": false,
"indent": ["error", 4],
"quotes": ["error", "single"],
"max-line-length": ["error", 120]
}
}
For code that requires external dependencies such as Mongo, Redis, Postgres, etc, ensure there is a docker-compose.yml file configured to run those dependencies. Do not assume that a developer has Mongo etc already installed. If the developer is a contractor with multiple clients it's often difficult or impossible for them to run such things on their bare metal.
In addition to using standard CI/CD such as Travis or Circle
- Use codecov.io to QA code coverage
- Use Greenkeeper to keep dependencies up to date
- For Javascript projects try and avoid using
gruntor other dated build systems and stick to basicnpmcommands within thepackage.jsonfile and use Webpack for bundling and for asset pipeline management - Avoid use of transpilers such as Babel for pure Node services such as API servers or libraries. They complicate release management. Instead ensure you are using a modern version of Node (9.7 or better) which includes almost all of the modern ES6 (and some ES7) features such as object spread notation out of the box.