diff options
Diffstat (limited to 'docs/REVIEWS.md')
| -rw-r--r-- | docs/REVIEWS.md | 89 |
1 files changed, 67 insertions, 22 deletions
diff --git a/docs/REVIEWS.md b/docs/REVIEWS.md index e1c657b333..d76f11f410 100644 --- a/docs/REVIEWS.md +++ b/docs/REVIEWS.md @@ -10,6 +10,7 @@ TVL Code Reviews - [Review process & approvals](#review-process--approvals) - [Registration](#registration) - [Submitting changes via email](#submitting-changes-via-email) + - [Gerrit for Github users](#gerrit-for-github-users) <!-- markdown-toc end --> @@ -34,7 +35,8 @@ 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/" +curl -Lo depot/.git/hooks/commit-msg https://cl.tvl.fyi/tools/hooks/commit-msg +chmod +x depot/.git/hooks/commit-msg ``` If you have a previous clone of the depot via HTTP you can use `git remote @@ -45,8 +47,11 @@ set-url` to update the origin URL and install the hook in the same way as above. 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`. +Instead of pushing changes to remote branches, all changes have to be pushed to +`refs/for/canon`. For each commit that is pushed there, a change request is +created automatically. + +Changes should usually be based on the remote `HEAD` (the `canon` branch). 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 @@ -73,7 +78,7 @@ git push origin git push origin HEAD:refs/for/canon%wip ``` -TIP: Every individual commit will become a separate change. We do not merge +TIP: Every individual commit will become a separate change. We do not squash 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. @@ -108,33 +113,37 @@ themselves**. ## Registration -If you would like to have an account on the Gerrit instance, follow these -instructions: +You may log into Gerrit using a GitHub, StackOverflow or GitLab.com account. + +If you would like to have a TVL-specific account on the Gerrit +instance, follow these instructions: 1. Be a member of `#tvl` on [hackint][]. 2. Clone the depot locally (via `git clone "https://cl.tvl.fyi/depot"`). 3. Create a user entry in our LDAP server in [ops/users][ops-users]. - We recommend using ARGON2 password hashes, which can be created - with the `slappasswd` tool if OpenLDAP was compiled with ARGON2 - support. - - For convenience, we provide a wrapper script for this that you can - build with `nix-build -A tools.hash-password` in a depot checkout. - Alternatively, if you have `direnv` installed, you can add the - depot to your allowlist and just run `hash-password` which should - be added to your `$PATH` by `direnv`. - - You can probably create ARGON2 hashes with other tools, but that is - your job to figure out. + The entry can be generated using [//web/pwcrypt](https://signup.tvl.fyi/). 4. Create a commit adding yourself (see e.g. [CL/2671](https://cl.tvl.fyi/c/depot/+/2671)) 5. Submit the commit via email (see below). ## Submitting changes via email +Please keep in mind this process is more complicated and requires more work from +both sides: -You can submit a patch via email to `depot@tazj.in` and it will be added to -Gerrit by a contributor. + - Someone needs to relay potential comments from Gerrit to you, you won't get + emails from Gerrit. + - Uploading new revisions needs to be done by the person sending it to Gerrit + on your behalf. + - If you decide to get a Gerrit account later on, existing CLs need to be + abandoned and recreated (as CLs can't change Owner). + This causing earlier reviews do be more disconnected, causing more churn. + +We provide local accounts and do SSO with various third-parties, so getting the +account should usually be low-friction. + +If you still decide differently, you can submit a patch via email to +`depot@tvl.su` and it will be added to Gerrit by a contributor. Create an appropriate commit locally and send it us using either of these options: @@ -143,12 +152,48 @@ Create an appropriate commit locally and send it us using either of these option * `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][]. +The email address is a [public inbox][]. + +## Gerrit for Github Users + +There is a walkthrough that describes [only the parts that differ +from Github][github-diff], although it does not cover [attention +sets][], which are important to understand. + +### Attention Sets + +The attention set of a CL is somewhat similar to the set of Github +users who have unread notifications for a PR. The "your turn" list +on the dashboard is similar to your unread notifications list in +Github. These similarities are only rough approximations, however. + +Unfortunately the rules for updating attention sets are very +different and complex. If you don't read and understand them, you +may end up leaving comments that nobody ever finds out about. Here +are a few unexpected features: + +- Voting on or replying to a CL will remove you from the attention + set. You can also do this by clicking on the gray chevron shape + next to your name. + +- If you comment on a merged or abandoned change without marking + your comment "unresolved", *nobody will be notified of your + comment*. If you want to the owner of a merged or abandoned + change to read your comment, you must mark it as "unresolved" or + manually add them to the attention set by hovering your mouse over + their name and clicking "add to attention set" + +There are many more [rules][attention-set-rules], which you should +read. + [Gerrit SSH]: https://cl.tvl.fyi/settings/#SSHKeys [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 [ops-users]: ../ops/users/default.nix -[public group]: https://groups.google.com/a/tazj.in/forum/?hl=en#!forum/depot +[public inbox]: https://inbox.tvl.su/depot/ [hackint]: https://hackint.org +[github-diff]: https://gerrit.wikimedia.org/r/Documentation/intro-gerrit-walkthrough-github.html +[attention sets]: https://gerrit-review.googlesource.com/Documentation/user-attention-set.html +[attention-set-rules]: https://gerrit-review.googlesource.com/Documentation/user-attention-set.html#_rules |