From 655682e23e134a5e63b1f2f62bad46ff249f8d07 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Fri, 21 Mar 2025 12:17:39 +0000 Subject: [PATCH] doc(contributing): place commit conventions upfront Change-Id: I26675ad0414209fdef723159576ef845f839e9a3 Reviewed-on: https://cl.snix.dev/c/snix/+/30227 Tested-by: besadii Reviewed-by: Florian Klink Autosubmit: Johannes Kirschbauer --- docs/CONTRIBUTING.md | 80 +------------------------ web/content/docs/guides/contributing.md | 57 +++++++++++++++++- 2 files changed, 59 insertions(+), 78 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 656ef9944..30863b11b 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -5,12 +5,9 @@ Contribution Guidelines **Table of Contents** - [Contribution Guidelines](#contribution-guidelines) - - [Before making a change](#before-making-a-change) - - [Commit messages](#commit-messages) - - [Commit content](#commit-content) - - [Code quality](#code-quality) - - [Builds & tests](#builds--tests) - - [Submitting changes](#submitting-changes) + - [Before making a change](#before-making-a-change) + - [Builds \& tests](#builds--tests) + - [Submitting changes](#submitting-changes) @@ -32,76 +29,6 @@ project. When in doubt - just ask! You can reach out to us via mail at [depot@tvl.su](mailto:depot@tvl.su) or on IRC. -## Commit messages - -All commit messages should be structured like this: - -``` -type(scope): Subject line with at most a 72 character length - -Body of the commit message with an empty line between subject and -body. This text should explain what the change does and why it has -been made, *especially* if it introduces a new feature. - -Relevant issues should be mentioned if they exist. -``` - -Where `type` can be one of: - -* `feat`: A new feature has been introduced -* `fix`: An issue of some kind has been fixed -* `docs`: Documentation or comments have been updated -* `style`: Formatting changes only -* `refactor`: Hopefully self-explanatory! -* `test`: Added missing tests / fixed tests -* `chore`: Maintenance work -* `subtree`: Subtree merges or updates, see also [Importing projects into depot][] - -And `scope` should refer to some kind of logical grouping inside of the project. - -It does not make sense to include the full path unless it aids in -disambiguating. For example, when changing the configuration of the host -`whitby` at `//ops/machines/whitby` it is enough to write `feat(whitby): ...`. - -Please take a look at the existing commit log for examples. - -This way of writing commit messages is known as [Conventional Commits][] and -should bring these advantages: - -* automatic creation of changelogs from commit messages -* disciplined commit hygiene: one focused change per commit -* filtering of commit history by type - -## Commit content - -Multiple changes should be divided into multiple git commits whenever possible. -Common sense applies. - -The fix for a single-line whitespace issue is fine to include in a different -commit. Introducing a new feature and refactoring (unrelated) code in the same -commit is not fine. - -`git commit -a` is generally **taboo**. - -In my experience making "sane" commits becomes *significantly* easier as -developer tooling is improved. The interface to `git` that I recommend is -[magit][]. Even if you are not yet an Emacs user, it makes sense to install -Emacs just to be able to use magit - it is really that good. - -For staging sane chunks on the command line with only git, consider `git add --p`. - -## Code quality - -This one should go without saying - but please ensure that your code quality -does not fall below the rest of the project. This is of course very subjective, -but as an example if you place code that throws away errors into a block in -which errors are handled properly your change will be rejected. - -In my experience there is a strong correlation between the visual appearance of -a code block and its quality. This is a simple way to sanity-check your work -while squinting and keeping some distance from your screen ;-) - ## Builds & tests All projects are built using [Nix][] to avoid "build pollution" via the user's @@ -128,6 +55,5 @@ review][] documentation. [magit]: https://magit.vc/ [Nix]: https://nixos.org/nix/ [code review]: ./REVIEWS.md -[Conventional Commits]: https://www.conventionalcommits.org [Importing projects into depot]: ./importing-projects.md [direnv]: https://direnv.net diff --git a/web/content/docs/guides/contributing.md b/web/content/docs/guides/contributing.md index 56baad206..ef2660da6 100644 --- a/web/content/docs/guides/contributing.md +++ b/web/content/docs/guides/contributing.md @@ -3,7 +3,7 @@ title: "Contributing" description: "" summary: "" date: 2025-03-14T14:14:35+01:00 -lastmod: 2025-03-14T14:14:35+01:00 +lastmod: 2025-03-21T14:12:42+00:00 draft: false weight: 12 toc: true @@ -124,3 +124,58 @@ $ git push origin HEAD:refs/for/canon%r=alice,cc=bob,l=Autosubmit+1,publish-comm [gerrit-for-github-users]: https://gerrit.wikimedia.org/r/Documentation/intro-gerrit-walkthrough-github.html [^1]: currently, `ssh-*-sk` keytypes are not supported, so use an `ssh-ed25519` key. [^2]: abbreviation for "change list", and the review unit in Gerrit. + +### Commit conventions + +#### Commit messages + +The following described way of writing commit messages is known as [Conventional Commits][] and +should bring these advantages: + +* automatic creation of changelogs from commit messages +* lean commits: one focused change per commit +* filtering of commit history by type + +All commit messages should be structured like this: + +``` +type(scope): Subject line with at most a 72 character length + +Body of the commit message with an empty line between subject and +body. This text should explain what the change does and why it has +been made, *especially* if it introduces a new feature. + +Relevant issues should be mentioned if they exist. +``` + +Where `type` can be one of: + +* `feat`: A new feature has been introduced +* `fix`: An issue of some kind has been fixed +* `docs`: Documentation or comments have been updated +* `style`: Formatting changes only +* `refactor`: Hopefully self-explanatory! +* `test`: Added missing tests / fixed tests +* `chore`: Maintenance work + +And `scope` should refer to some kind of logical grouping inside of the project. + +It does not make sense to include the full path unless it aids in +disambiguating. For example, when changing `/snix/eval/src/lib.rs` it is enough to write `feat(eval): ...`. + +Please take a look at the existing [commit history][] for examples. + +#### Commit content + +Multiple changes should be divided into multiple git commits whenever possible. +Common sense applies. + +#### Code quality + +This one should go without saying - but please ensure that your code quality +does not fall below the rest of the project. This is of course very subjective, +but as an example if you place code that throws away errors into a block in +which errors are handled properly your change might be rejected. + +[commit history]: https://git.snix.dev/snix/snix/commits/branch/canon +[Conventional Commits]: https://www.conventionalcommits.org