Skip to content

Commit

Permalink
docs: Updates a few process documents
Browse files Browse the repository at this point in the history
  • Loading branch information
cvilas committed Feb 27, 2024
1 parent 54de6f1 commit 5b741ae
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 71 deletions.
34 changes: 22 additions & 12 deletions process/code_reviews.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
# Code Review Guidelines

Follow [Google Code Review guidelines](https://google.github.io/eng-practices/review/).
### To all team members

Additionally,
- Set aside time in your sprint to account for unplanned review requests from your colleagues.

1. All developers: Set aside time in your sprint to account for unplanned review requests from your colleagues.
2. Communicate:
- Reviewers: When requested for a review, make your availability known.
- Authors: Please give reviewers a heads-up on what the MR is about (if not in the MR description already).
- In summary, make expectations clear.
3. Reviewer mindset:
- Make actionable suggestions
- Focus on API clarity and user experience. Less than perfect internal details could be improved in future MRs
4. Empathise: When asking for a review, you are asking for unplanned time from your colleague, who may be busy on their
own tasks. Work with them at a personal level to get your MR through in a timely manner.
### To MR authors

- Please give reviewers a heads-up on what the MR is about (if not in the MR description already).
- Empathise: You are asking for unplanned time from your colleague, who may be busy on their own tasks. Work with them personally to get your MR through in a timely manner.

### To reviwers

- Make your availability known when your review is requested. Clarify expected response time.
- In reviewing an MR
- Make actionable suggestions
- Focus on API clarity and user experience. Less than perfect internal details could be improved in future MRs
- Expect production-quality code, even at low TRLs. Reject MR if:
- The work is not properly documented as per coding guidelines.
- The work disregards basic software design and engineering principles (eg: [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself), [SOLID](https://en.wikipedia.org/wiki/SOLID))
- The implementation is inconsistent with system architecture znd long term vision for the product
- The code is not formatted as per style guidelines
- The code is not compliant with language usage rules (i.e. generates linter warnings)
- The code generates compiler warnings

For additional guidance, follow [Google Code Review guidelines](https://google.github.io/eng-practices/review/).
21 changes: 11 additions & 10 deletions process/commit_messages.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
# Guide to commit messages

- Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/)
- Rationale: Consistent format; human- and machine-parsable to generate meaningful changelogs between releases
- Structure
Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) for consistent format that is human- and machine-parsable to generate meaningful changelogs between releases

## Structure

```text
<type>[optional scope]: <description>
[optional body]
[optional body: Provides additional implementation detail if necessary]
[optional footer(s)]
[optional footer(s): Reference to issue tracker]
```

- `type` can be of `build`, `chore`, `docs`, `feat`, `fix`, `perf`, `refactor`, `revert`, `style`, `test`

- Example (`!` signifies breaking change):
## Example

(`!` signifies breaking change)

```text
feat(api)!: send an email to the customer when a product is shipped
feat(api)!: Sends an email to the customer when a product is shipped
Body of the message explaining the change in
multiple lines.
Adds integration with email client and customer contact information backend to autogenerate and dispatch email with shipping information.
Reference: https://github.com/cvilas/guidance/issues/7
Reference: https://github.com/<organisation>/<project>/issues/7
```
65 changes: 28 additions & 37 deletions process/developers_code.md → process/developer_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@

> "Code is a way you treat your coworkers." - Michael Feathers.
1. Design on paper first
1. Follow branch and merge model for integrating code changes
1. Follow coding guidelines for correctness, style, and organisation
1. Document the work for future self, peers, and for posterity

## Design on paper first
## Follow branch and merge model for integrating code changes

> Writing is Thinking
Without committing to and aligning with _long-term_ vision for the product, it is impossible to develop software that is coherent over the long term. So review product concept documents first.

Resist the urge to start implementing without a _written_ design document, even if elementary and informal. This should, at a minimum, include:

- Assumptions made in order to arrive at the design
- The inputs, outputs, states and the intended behavior
- Interface specification and interaction with other components
- Failure modes and recovery mechanisms
- Create a new branch from `main` to work on your feature or fix
- Follow coding guidelines (see subsequent section)
- Document your work (see subsequence section)
- When work is completed,
- Tidy up commit history with `git rebase`. Follow guidelines for [commit messages](commit_messages.md).
- Test your work in simulation and real hardware as appropriate.
- Raise a merge request and have it [peer reviewed](code_reviews.md) before merging.
- Follow a continuous delivery model to release feature into production. See [release cadence](release_cadence.md).

It's acceptable for specifications to evolve with implementation, as we uncover previously overlooked issues. Embrace iterative development driven by field testing and feedback.

Expand All @@ -31,31 +29,7 @@ It's acceptable for specifications to evolve with implementation, as we uncover
- In coding _style_, be consistent. Follow [ROS C++ Style Guide](https://wiki.ros.org/CppStyleGuide). (example [.clang-format](.clang-format) configuration )
- Aim to implement code that is [correct by construction](correct_by_construction.md).
- Employ [third party software](third_party.md) sparingly, and with great caution and skepticism
- Follow a consistent format for [commit messages](commit_messages.md)
- Organise the implementation into logical 'modules', each with its own unit tests, example programs, and documentation. See [modules](#modules)
- When work is completed, raise a pull request and assign peers for [code review](code_reviews.md)

Anything that goes in the `main` branch is 'production code' even at low TRLs. Repo maintainers should reject pull requests if

- The work is not properly [documented](#document-the-work)
- The work disregards basic software design and engineering principles (eg: [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself), [SOLID](https://en.wikipedia.org/wiki/SOLID))
- The design is inconsistent with long-term vision for the product
- The code is not formatted as per style guidelines
- The code is not compliant with language usage rules (i.e. generates linter warnings)
- The code generates compiler warnings

## Document the work

> 'Code _is_ documentation'. However it is only an _implementation_ document, not a _design_ document.
Deviation from the design intent, inadvertent or deliberate, is usually a defect. This is impossible to detect without
corresponding design documentation.

- Colocate design documents and datasheets within codebase to make the codebase self-sufficient.
- Prefer simple data formats (`txt`, `md`)
- Implemented a new algorithm? Cite the source, or place relevant publications or even a copy of handwritten notes in the docs folder. The point is to sufficiently document the problem and the approach taken to solve it without forcing readers to reverse engineer the code or the implementer's thought process
- Provide code snippets and example programs to demonstrate intended usage
- Unless it's trivially obvious, document the API and parameters (eg: valid ranges, if it cannot be enforced in code, for instance, with `asserts`). Follow the doxygen convention in doing so.

### Modules

Expand All @@ -77,4 +51,21 @@ A suggested directory structure is as follows:
- tests (unit tests)
- examples (demonstrative examples)
- docs (datasheets, design docs)
- scripts (tools, etc)
- scripts (tools, etc)

## Document the work

> 'Code _is_ documentation'. However it is only an _implementation_ document, not a _design_ document.
Deviation from the design intent, inadvertent or deliberate, is usually a defect. This is impossible to detect without corresponding design documentation.

- Design documentation includes
- Assumptions made in order to arrive at the design
- The inputs, outputs, states and the intended behavior
- Interface specification and interaction with other components
- Failure modes and recovery mechanisms
- Colocate design documents and datasheets within codebase to make the codebase self-sufficient.
- Prefer simple data formats (`txt`, `md`)
- Implemented a new algorithm? Cite the source, or place relevant publications or even a copy of handwritten notes in the docs folder. Do not force readers to reverse engineer the code or the implementer's thought process
- Provide code snippets and example programs to demonstrate intended usage
- Unless it's trivially obvious, document the API and parameters (eg: valid ranges, if it cannot be enforced in code, for instance, with `asserts`). Follow the doxygen convention in doing so.
2 changes: 1 addition & 1 deletion process/process.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Process

- Follow [developers' code](developers_code.md) for writing software
- Follow the [developer's guide](developer_guide.md) for writing software
- Settle on a [release cadence](release_cadence.md) that encourages continuous delivery
- Set up the build system to generate and deploy artifacts in one step. Rationale: Enables frictionless deployment for testing and release.
- Maintain a bug database. Minimally, a bug report shall contain: steps to reproduce bug, expected behaviour, observed behaviour, who is it assigned to, fix schedule
Expand Down
25 changes: 16 additions & 9 deletions process/release_cadence.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,31 @@

![Release Cadence](../media/release_cadence.png)

Follow a 'rolling release' development model (also called 'continuous delivery) that aims to release software to production bi-weekly. Every release candidate (RC) goes through two system-level validation and verification (V&V) tests, V&V1 and V&V2, conducted a week apart, before it is released.
## Principles

- On the last day of a two-week Sprint, a new RC branch is generated, typically from the head of `main` branch.
- V&V process begins the following Monday (start of next Sprint), with V&V-1 and V&V-2 spaced one week apart.
- If V&V-1 passes, the software is released to production, and V&V-2 is skipped.
- If V&V-1 fails, the team shall triage and aim to resolve issues before V&V-2. Changes are integrated into the RC branch following the [hotfix procedure](#hotfixing-a-release-branch).
- If V&V-2 passes, the software is released to production. If V&V-2 fails, the release RC is abandoned and the release skipped for the current cycle.
- The `main` branch is always 'green' (i.e. functional)
- Releases are created from `main` branch and nominally delivered to production bi-weekly.
- Every release candidate (RC) gets two validation and verification (V&V) tests, before release.

Note that if V&V-2 is skipped, we shall not expedite the next release; the next V&V-1 shall be scheduled as usual at the start of next Sprint. This fixed cadence gives each release at least two weeks to accumulate hours in production, and balances the interests of both the Development team (who want to see their features in production asap) and the Operations team (who value stability in production environment).
## Procedure

## Creating a release branch
- On the last day of a two-week Sprint, generate a new RC branch from the head of `main` branch.
- Start V&V-1 the following Monday (start of next Sprint). V&V-1 and V&V-2 spaced one week apart.
- If V&V-1 passes, release software to production. Skip V&V-2.
- If V&V-1 fails, attempt to triage and resolve issues before V&V-2. Integrate any changes into the RC branch following the [hotfix procedure](#hotfixing-a-release-branch).
- If V&V-2 passes, release software to production.
- If V&V-2 fails, abandon the RC and skip the release for the current cycle.

Note that if V&V-2 is skipped, we shall not expedite the next release; the next V&V-1 shall be scheduled as usual at the start of next Sprint. This fixed cadence gives each release at least two weeks to accumulate hours in production, and balances the interests of both the development team (who want to see their features in production asap) and the operations team (who value stability in production environment).

### Creating a release branch

- Create release branch from `main` branch. Call it `release-<version>`, where `version` is the version identifier.
- Mark the release branch protected so that only maintainers can modify this branch
- Build and package up the artifacts
- Publish

## Hotfixing a release branch
### Hotfixing a release branch

This procedure is for publishing a hotfix (eg: bug fix) for a release candidate (RC) or software in production.

Expand Down
7 changes: 5 additions & 2 deletions process/third_party.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
- Certainly avoid third party libraries as a way to outsource core competencies.
- Integrate a third-party library into the codebase _only when_ it does not force design compromises.

Rationale 1: Design core components from first principles, building only what we need, following our design guidelines. Re-invent the wheel. This approach results in a system that is simpler, easier to understand, extend and maintain.
### Rationale

Rationale 2: We _own_ any third-party code we integrate, i.e., we become responsible for its bug fixes and maintenance. Customer operations cannot stop and wait for original authors of a library to fix the software upstream. Better to write and fix your own bugs than deal with others'.
1. Design core components from first principles, building only what we need, following our design guidelines. Re-invent the wheel. This approach results in a system that is simpler, easier to understand, extend and maintain.
1. We _own_ any third-party code we integrate, i.e., we become responsible for its bug fixes and maintenance. Customer operations cannot stop and wait for original authors of a library to fix the software upstream. Better to write and fix your own bugs than deal with others'.

## HowTo

If the benefits of using third-party libraries outweigh the aforementioned drawbacks, then:

Expand Down

0 comments on commit 5b741ae

Please sign in to comment.