diff options
Diffstat (limited to 'docs')
| -rw-r--r-- | docs/CODE_OF_CONDUCT.md | 29 | ||||
| -rw-r--r-- | docs/CONTRIBUTING.md | 120 | ||||
| -rw-r--r-- | docs/REVIEWS.md | 199 | ||||
| -rw-r--r-- | docs/designs/SPARSE_CHECKOUTS.md | 43 |
4 files changed, 391 insertions, 0 deletions
diff --git a/docs/CODE_OF_CONDUCT.md b/docs/CODE_OF_CONDUCT.md new file mode 100644 index 000000000000..0e46bbedb0cd --- /dev/null +++ b/docs/CODE_OF_CONDUCT.md @@ -0,0 +1,29 @@ +A SERMON ON ETHICS AND LOVE +=========================== + +One day Mal-2 asked the messenger spirit Saint Gulik to approach the +Goddess and request Her presence for some desperate advice. Shortly +afterwards the radio came on by itself, and an ethereal female Voice +said **YES?** + +"O! Eris! Blessed Mother of Man! Queen of Chaos! Daughter of Discord! +Concubine of Confusion! O! Exquisite Lady, I beseech You to lift a +heavy burden from my heart!" + +**WHAT BOTHERS YOU, MAL? YOU DON'T SOUND WELL.** + +"I am filled with fear and tormented with terrible visions of pain. +Everywhere people are hurting one another, the planet is rampant with +injustices, whole societies plunder groups of their own people, +mothers imprison sons, children perish while brothers war. O, woe." + +**WHAT IS THE MATTER WITH THAT, IF IT IS WHAT YOU WANT TO DO?** + +"But nobody Wants it! Everybody hates it." + +**OH. WELL, THEN *STOP*.** + +At which moment She turned herself into an aspirin commercial and left +The Polyfather stranded alone with his species. + +SINISTER DEXTER HAS A BROKEN SPIROMETER. diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md new file mode 100644 index 000000000000..70410d290e7d --- /dev/null +++ b/docs/CONTRIBUTING.md @@ -0,0 +1,120 @@ +Contribution Guidelines +======================= + +<!-- markdown-toc start - Don't edit this section. Run M-x markdown-toc-refresh-toc --> +**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) + +<!-- markdown-toc end --> + +This is a loose set of "guidelines" for contributing to the depot. Please note +that we will not accept any patches that don't follow these guidelines. + +Also consider the [code of conduct](./CODE_OF_CONDUCT.md). No really, +you should. + +## Before making a change + +Before making a change, consider your motivation for making the change. +Documentation updates, bug fixes and the like are *always* welcome. + +When adding a feature you should consider whether it is only useful for your +particular use-case or whether it is generally applicable for other users of the +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`: Operations involving `git subtree` + +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. + +## 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 +environment. + +If you have Nix installed and are contributing to a project tracked in this +repository, you can usually build the project by calling `nix-build -A +path.to.project`. + +For example, to build a project located at `//tools/foo` you would call +`nix-build -A tools.foo` + +If the project has tests, check that they still work before submitting your +change. + +## Submitting changes + +The code review & change submission process is described in the [code +review][] documentation. + +[magit]: https://magit.vc/ +[Nix]: https://nixos.org/nix/ +[code review]: ./REVIEWS.md diff --git a/docs/REVIEWS.md b/docs/REVIEWS.md new file mode 100644 index 000000000000..d76f11f41074 --- /dev/null +++ b/docs/REVIEWS.md @@ -0,0 +1,199 @@ +TVL Code Reviews +================ + +<!-- markdown-toc start - Don't edit this section. Run M-x markdown-toc-refresh-toc --> +**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) + - [Gerrit for Github users](#gerrit-for-github-users) + +<!-- markdown-toc end --> + + +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. + +First, [tell Gerrit][Gerrit SSH] about your SSH keys. + +Then, to make using Gerrit 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" +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 +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. + +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 +of your commit and want to submit it for review, you push it to a git ref called +`refs/for/canon`. This designates the commits as changelists (CLs) targeted for +the `canon` branch. + +Sending a change for review is done by pushing to a special target. You can set +this to be the default push target through your git configuration: + +``` +git config remote.origin.url "ssh://$USER@code.tvl.fyi:29418/depot" +git config remote.origin.push HEAD:refs/for/canon +``` + +Then, after making your change, push to the default, or to a special target: + +``` +Example: +git commit -m 'docs(REVIEWS): Fixed all the errors in the reviews docs' +git push origin + +# Uploading a work-in-progress CL: +git push origin HEAD:refs/for/canon%wip +``` + +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. + +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/<username>` 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 + +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]. + + 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: + + - 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: + +* `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 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 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 diff --git a/docs/designs/SPARSE_CHECKOUTS.md b/docs/designs/SPARSE_CHECKOUTS.md new file mode 100644 index 000000000000..820cb2c5861c --- /dev/null +++ b/docs/designs/SPARSE_CHECKOUTS.md @@ -0,0 +1,43 @@ +NOTE: This proposal is archived. We run `josh` instead, and long-term +might want to integrate per-target dependency analysis with josh's +workspace functionality. + +------------- + +Below is a prototype for a script to create Git sparse checkouts of the depot. +The script below works today with relatively recent versions of git. + +Open items: + + - Read-increment-write the checkout ID from a file in .git. + - `NICE_CHECKOUT_ROOT` should be a git configuration value. + - `tvl-get-depends` will be a script that contacts the build farm and asks for + the closure of a given source directory, using [depot-scan]. + +```bash +DEPOT_ROOT="${depot.path}" +XDG_DATA_HOME="${XDG_DATA_HOME:-$HOME/.local/share}" +CLIENT_ROOT="$XDG_DATA_HOME/tvlc/clients" +NICE_CHECKOUT_ROOT="$HOME/tvlc" +CHECKOUT_ID=1 +CHECKOUT_NAME=myclient # command line variables + +assertAbsolutePath "$CLIENT_ROOT" + +mkdir "$CLIENT_ROOT"/"$CHECKOUT_ID" +ln -s "$CLIENT_ROOT"/"$CHECKOUT_ID" "$NICE_CHECKOUT_ROOT"/"$CHECKOUT_NAME" + +cd "$DEPOT_ROOT" +git worktree add --no-checkout -b "tvlc-$CHECKOUT_ID" "$CLIENT_ROOT/$CHECKOUT_ID/" canon +# BUG: git not creating the /info/ subdir +mkdir "$DEPOT_ROOT"/.git/worktrees/"$CHECKOUT_ID"/info + +cd "$CLIENT_ROOT/$CHECKOUT_ID" +git sparse-checkout init --cone +git sparse-checkout set "$TARGET_DIR" nix/readTree overrides +tvl-get-depends "$TARGET_DIR" | xargs git sparse-checkout add + +cd "$NICE_CHECKOUT_ROOT"/"$CHECKOUT_NAME" +``` + +[depot-scan]: ../users/edef/depot-scan.nix |