dft street manager doc 0000 code quality

Code Quality

Code Review

Code reviews provide immediate feedback on style, clarity, robustness and other non-functional qualities of the code delivered.

Code reviews are the responsibility of everyone writing and commiting code and all code must be reviewed as part of the definition of done.

This is a peer activity - no one person should act as the bottle-neck for review completion - and everyone will be expected to contribute ideas for improvement of the code base.

Judiciously apply the Boy Scout Rule

Process

Code reviews are completed using the GitHub pull request mechanism, with the Pull Request acting as the gateway for reviews - code must not be merged until a code review has been completed and any comments have been adequately addressed.

Collaborative code review is now part of the standard GitHub feature set.

See the following video for a summary: Github Code Review

Checklist

A checklist is an effective way to make sure the contents of a pull request and code review are consistent. Omissions are difficult defects to pick up - it's hard to review something which isn't there.

It is expected that the checklist for code reviews should evolve over time to add or remove items which are deemed to make a difference to the quality of the outcome.

This checklist is for the benefit of the primary reviewer - aka YOU - the person submitting the pull request. Have you considered the following:

• Unit tests written and passing
• Coding standards adhered to
• Coding conventions adhered to (well factored and maintainable)
• Error Handling
• If the API has been altered has the documentation been updated?
• Security considered (Security Overview)
• Performance considered
• Appropriate use of logging
• README updated?
• Updates required for deployment?

Coding Standards

JavaScript Coding Standards

All JavaScript code must conform to the Standard JS style guide.

Any new JavaScript components developed should include a step to lint the source files as part of the package.json test script definition, before any unit tests are run:

E.g.

"scripts": { "test": "node_modules/.bin/standard" }

Standard should also be enabled for your development IDE.

A list of plugins for all common editors can be found here: Text Editor Plugins

Conventions

For conventions and other recommendations we will use the AirBnB public standard where they do not conflict with StandardJS.

Naming

Routes should be named after the page they represent (e.g. the route handling Create New Work should be named create-new-work.js

Database Conventions

See Data model and standards

API Design Principles

See here

Github

See details of feature branch workflow within Solution Architecture

Use Atomic Commits

As a general rule we should strive to keep commits atomic.

That means any given commit should encapsulate a single feature, fix or improvement.

For example, don't mix bug fixes with features. Try to keep the scope of the commit to a single purpose. This will help greatly with code reviews, rollback of changes and maintaining sanity in the repository log.

Effective Commit Messages

Good commit messages are an effective way to communicate context about a change to other members of the development team.

Let's adopt the following conventions:

• The first line is the subject
• All other lines contain the body
• Separate subject from body with a blank line
• Try to keep the subject to 60 characters max (GitHub truncates subjects longers than 69 chars with ellipsis)
• The subject should be a proper sentence, so capitalise it
• Do not end the subject line with a period
• Use the body to explain what and why versus how
• Wrap the body text at 72 characters

Full background post to these guidelines here: Git Commits

Branch Naming Conventions

Features: feature/sw-{story id}/short-description

E.g. feature/sw-118/see-works-overdue-reinstatement

Chores: chore/short-description

E.g. chore/fix-docker-compose-public