From b27239b60a5babcdb71999e9c6a5da68231549a8 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Wed, 17 Jun 2020 03:23:37 +0100 Subject: docs(REVIEWS): Add documentation for code reviews in Gerrit Adds a document with information about the review process, how to get an account, how to submit patches via email and so on. Change-Id: I7c266e063ef53ab6aa79dd2582bc0e4fa8dcf5c0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/446 Reviewed-by: riking --- docs/CONTRIBUTING.md | 15 ++---- docs/REVIEWS.md | 129 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 docs/REVIEWS.md (limited to 'docs') diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 61c485ca5f..7f2c661a5b 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -10,6 +10,7 @@ Contribution Guidelines - [Commit content](#commit-content) - [Code quality](#code-quality) - [Builds & tests](#builds--tests) + - [Submitting changes](#submitting-changes) @@ -110,17 +111,9 @@ change. ## Submitting changes -Development is primarily tracked at [cl.tvl.fyi](https://cl.tvl.fyi) using -Gerrit. If you have an account, use the standard Gerrit workflows to submit a -change. - -If you do not have an account, you can alternatively submit a patch -via email. Create an appropriate commit locally and send it to -`depot@tazj.in` using either `git send-email` or `git format-patch`. -The email address is a [public group][]. - -Patches submitted via email will be added to Gerrit by a contributor. +The code review & change submission process is described in the [code +review][] documentation. [magit]: https://magit.vc/ [Nix]: https://nixos.org/nix/ -[public group]: https://groups.google.com/a/tazj.in/forum/?hl=en#!forum/depot +[code review]: ./REVIEWS.md diff --git a/docs/REVIEWS.md b/docs/REVIEWS.md new file mode 100644 index 0000000000..d455b75bad --- /dev/null +++ b/docs/REVIEWS.md @@ -0,0 +1,129 @@ +TVL Code Reviews +================ + + +**Table of Contents** + +- [TVL Code Reviews](#tvl-code-reviews) + - [Gerrit setup](#gerrit-setup) + - [Gerrit workflows](#gerrit-workflows) + - [Review process & approvals](#review-process--approvals) + - [Registration](#registration) + - [Submitting changes via email](#submitting-changes-via-email) + + + + +This document describes the TVL code review process & tooling. If you are +looking for general contribution guidelines, please look at the [general +contribution guidelines](./CONTRIBUTING.md). + +All changes are tracked at [cl.tvl.fyi](https://cl.tvl.fyi) using Gerrit. See +[Registration](#registration) for information on how to register an account. + +## Gerrit setup + +Gerrit uses the concept of change IDs to track commits across rebases and other +operations that might change their hashes, and link them to unique changes in +Gerrit. + +To make this process smooth for users, the repository should be cloned and a +commit hook should be installed as follows: + +``` +git clone "ssh://$USER@code.tvl.fyi:29418/depot" +scp -p -P 29418 $USER@code.tvl.fyi:hooks/commit-msg "depot/.git/hooks/" +``` + +If you have a previous clone of the depot via HTTP you can use `git remote +set-url` to update the origin URL and install the hook in the same way as above. + +## Gerrit workflows + +The developer workflow on Gerrit is quite different from what GitHub-users are +used to. + +The depot does not have branches (other than Gerrit's internal metadata refs) +and all development happens at `HEAD`. + +Every time you create a new commit the change hook will insert a unique +`Change-Id` tag into the commit message. Once you are satisfied with the state +of your commit and want to submit it for review, you push it to a git ref called +`refs/for/master`. This designates the commits as changelists (CLs) targeted for +the `master` branch. + +``` +Example: +git commit -m 'docs(REVIEWS): Fixed all the errors in the reviews docs' +git push origin HEAD:refs/for/master +``` + +TIP: Every individual commit will become a separate change. We do not merge +related commits, but instead submit them one by one. Be aware that if you are +expecting a different behaviour and attempt something like an unsquashed subtree +merge, you will produce a *lot* of CLs. This is strongly discouraged. + +During your review, the reviewer(s) might ask you to make changes. You can +simply amend your commit(s) and push to the same ref. Gerrit will automatically +update your changes. + +Read more about the Gerrit workflow in the [Gerrit walkthrough][]. + +## Review process & approvals + +Each user has the ability to create their own users directory in +`//users/` in which they can submit code without review from other +contributors (they will still need to +2 their own changes, and the initial +check-in of the `OWNERS` file needs to be reviewed). + +You can set a directory like this up for yourself by proposing a change similar +to [CL/246](https://cl.tvl.fyi/c/depot/+/246). + +For all paths outside of `//users`, code review is required. We have no strict +guidelines about the review process itself, as we're not a megacorp, but we have +formalised checks before submitting: + +* At least one person who is [an owner][OWNERS] of the codepath must have given + a +2 review +* The commit message must conform to our [guidelines][] +* No code review comments must be left unresolved + +If all these conditions are fulfilled, the **change author submits their change +themselves**. + +## Registration + +If you would like to have an account on the Gerrit instance, follow these +instructions: + +1. Be a member of `##tvl`. +2. Clone the depot locally. +3. Create a user entry in our LDAP server, in [`contents.ldif`][]. You create + your password hash by using `slappasswd`, which ships in the `openldap` + package on most distributions. + + On systems with Nix systems you can use `nix-shell -p openldap` to get access + to the command. +4. Create a commit adding yourself (see e.g. + [CL/223](https://cl.tvl.fyi/c/depot/+/223)). +5. Submit the commit via email (see below). + +## Submitting changes via email + +You can submit a patch via email to `depot@tazj.in` and it will be added to +Gerrit by a contributor. + +Create an appropriate commit locally and send it us using either of these options: + +* `git format-patch`: This will create a `.patch` file which you should email to + us. +* `git send-email`: If configured on your system, this will take care of the + whole emailing process for you. + +The email address is a [public group][]. + +[Gerrit walkthrough]: https://gerrit-review.googlesource.com/Documentation/intro-gerrit-walkthrough.html +[OWNERS]: https://cl.tvl.fyi/plugins/owners/Documentation/config.md +[guidelines]: ./CONTRIBUTING.md#commit-messages +[`contents.ldif`]: https://code.tvl.fyi/tree/ops/nixos/tvl-slapd/contents.ldif +[public group]: https://groups.google.com/a/tazj.in/forum/?hl=en#!forum/depot -- cgit 1.4.1