Skip to content

Commit

Permalink
Update the contributing guide with our new git-cliff setup
Browse files Browse the repository at this point in the history
Co-authored-by: Ivan Enderlin <[email protected]>
  • Loading branch information
poljar and Hywan committed Oct 10, 2024
1 parent 176fb31 commit ff2323c
Showing 1 changed file with 93 additions and 26 deletions.
119 changes: 93 additions & 26 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,43 +29,110 @@ integration tests that need a running synapse instance. These tests reside in
[README](./testing/matrix-sdk-integration-testing/README.md) to easily set up a
synapse for testing purposes.

## Commit messages and PR title guidelines

Ideally, a PR should have a *proper title*, with *atomic logical commits*, and each commit
should have a *good commit message*.
## Pull requests

An *atomic logical commit* is one that is ideally small, can be compiled in isolation, and passes
tests. This is useful to make the review process easier (help your reviewer), but also when running
bisections, helping identifying which commit introduced a regression.
Ideally, a PR should have a *proper title*, with *atomic logical commits*, and
each commit should have a *good commit message*.

A *good commit message* should be composed of:
A *proper PR title* would be a one-liner summary of the changes in the PR,
following the same guidelines of a good commit message, including the
area/feature prefix. Something like `FFI: Allow logs files to be pruned.` would
be a good PR title.

- a prefix to indicate which area/feature is related by the commit
- a short description that would give sufficient context for a reviewer to guess what the commit is
about.
(An additional bad example of a bad PR title would be `mynickname/branch name`,
that is, just the branch name.)

Examples of commit messages that aren't so useful:
# Writing changelog entries

- “add new method“
- “enhance performance“
- “fix receipts“
We aim to maintain clear and informative changelogs that accurately reflect the
changes in our project. This guide will help you write useful changelog entries
using git-cliff, which fetches changelog entries from commit messages.

Examples of good commit messages:
## Commit message format

- “ffi: Add new method `frobnicate_the_foos`
- “indexeddb: Break up the request inside `get_inbound_group_sessions`
- “read_receipts: Store receipts locally, fixing #12345
Commit messages should be formatted as Conventional Commits. In addition, some
git trailers are supported and have special meaning (see below).

A *proper PR title* would be a one-liner summary of the changes in the PR, following the
same guidelines of a good commit message, including the area/feature prefix. Something like
`FFI: Allow logs files to be pruned.` would be a good PR title.
### Conventional commits

(An additional bad example of a bad PR title would be `mynickname/branch name`, that is, just the
branch name.)
Conventional Commits are structured as follows:

Having good commit messages and PR titles also helps with reviews, scanning the `git log` of
the project, and writing the [*This week in
Matrix*](https://matrix.org/category/this-week-in-matrix/) updates for the SDK.
```
<type>(<scope>): <short summary>
```

The type of changes which will be included in changelogs is one of the following:

* `feat`: A new feature
* `fix`: A bug fix
* `doc`: Documentation changes
* `refactor`: Code refactoring
* `perf`: Performance improvements
* `ci`: Changes to CI configuration files and scripts

The scope is optional and can specify the area of the codebase affected (e.g.,
olm, cipher).

### Changelog trailer

In addition to the Conventional Commit format, you can use the `Changelog` git
trailer to specify the changelog message explicitly. When that trailer is
present, its value will be used as the changelog entry instead of the commit's
leading line. The `Breaking-Change` git trailer can be used in a similar manner
if the changelog entry should be marked as a breaking change.


#### Example commit message

```
feat: Add a method to encode Ed25519 public keys to Base64
This patch adds the `Ed25519PublicKey::to_base64()` method, which allows us to
stringify Ed25519 and thus present them to users. It's also commonly used when
Ed25519 keys need to be inserted into JSON.
Changelog: Added the `Ed25519PublicKey::to_base64()` method which can be used to
stringify the Ed25519 public key.
```

In this commit message, the content specified in the `Changelog` trailer will be
used for the changelog entry.

### Security fixes

Commits addressing security vulnerabilities must include specific trailers for
vulnerability metadata. These commits are required to include at least the
`Security-Impact` trailer to indicate that the commit is a security fix.

Security issues have some additional git-trailers:

* `Security-Impact`: The magnitude of harm that can be expected, i.e. low/moderate/high/critical.
* `CVE`: The CVE that was assigned to this issue.
* `GitHub-Advisory`: The GitHub advisory identifier.

Example:

```
fix(crypto): Use a constant-time Base64 encoder for secret key material
This patch fixes a security issue around a side-channel vulnerability[1]
when decoding secret key material using Base64.
In some circumstances an attacker can obtain information about secret
secret key material via a controlled-channel and side-channel attack.
This patch avoids the side-channel by switching to the base64ct crate
for the encoding, and more importantly, the decoding of secret key
material.
Security-Impact: Low
CVE: CVE-2024-40640
GitHub-Advisory: GHSA-j8cm-g7r6-hfpq
Changelog: Use a constant-time Base64 encoder for secret key material
to mitigate side-channel attacks leaking secret key material.
```

## Review process

Expand Down

0 comments on commit ff2323c

Please sign in to comment.