From 6d4622969f97395a1bedc3ccbde284a143088cf7 Mon Sep 17 00:00:00 2001
From: Gnuxie <50846879+Gnuxie@users.noreply.github.com>
Date: Mon, 15 Apr 2024 17:50:05 +0100
Subject: [PATCH] contributing documentation overhaul. (#344)

* contributing documentation overhaul.

* hm

* declutter development stuff from readme.

* hmm

* broken link
---
 CONTRIBUTING.md                 | 197 ++++++++++++--------------------
 README.md                       |  64 +----------
 docs/code-style.md              |  68 +++++++++++
 docs/development-environment.md | 162 ++++++++++++++++++++++++++
 docs/development.md             |  16 +++
 docs/triaging.md                |  55 +++++++++
 6 files changed, 382 insertions(+), 180 deletions(-)
 create mode 100644 docs/code-style.md
 create mode 100644 docs/development-environment.md
 create mode 100644 docs/development.md
 create mode 100644 docs/triaging.md

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 574984ea..793745d8 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,140 +1,95 @@
-# Contributing code to Draupnir
+# Contributing to Draupnir
 
-## How to contribute
+## Welcome
 
-The preferred and easiest way to contribute changes to Matrix is to fork the
-relevant project on github, and then [create a pull request](
-https://help.github.com/articles/using-pull-requests/) to ask us to pull
-your changes into our repo.
+Hi, we're that you are considering to contribute to Draupnir.
+We want this process to be as welcoming as possible, no matter your
+experience level, or the kind of contribution you want to make.
 
-We use Github Actions for continuous integration.
-If your change breaks the build, this will be shown in GitHub, so
-please keep an eye on the pull request for feedback.
+If you believe that your experience is anything but that, please let
+us know!
 
-## How Draupnir works
+Do not worry about following the guidance in this document to the
+letter, we'd much rather you get involved than avoid doing so
+because of a technicality. Keep this in mind throughout.
 
-You should read the [context document](./docs/context.md).
+## Getting Started
 
+What kind of contribution are you trying to make?
 
-## Development
+If you are looking to document an issue, request a feature, develop
+a feature, please proceed into the [Issue](#issue) section.
 
-To run unit tests in a local development environment, you can use `yarn test`
-and `yarn lint`.
+If you are looking to develop or contribute a fix, feature,
+or documentation for an existing issue, please proceed to the
+[Fixing or implementing an existing issue](#fixing-or-implementing-an-existing-issue) section.
 
-### matrix-protection-suite
+### Issue
 
-While not necessary, some changes will require you to make changes to the
-[matrix-protection-suite](https://github.com/Gnuxie/matrix-protection-suite)
-and the associated backend for the matrix-bot-sdk: [matrix-protection-suite-for-matrix-bot-sdk](https://github.com/Gnuxie/matrix-protection-suite-for-matrix-bot-sdk).
+If you can, just open the issue on the repository and we'll see it
+and come speak to you.
 
-You should clone these locally and then link them by using
-`yarn link` in each directory followed by `yarn link matirx-protection-suite matrix-protection-suite-for-matrix-bot-sdk` within Draupnir.
+Alternatively, if you aren't comfortable doing so or can't phrase
+the problem or feature, then come speak to us in our support room.
+We'll probably end up creating the issue for you!
 
-You may also need to `yarn add --dev "matrix-bot-sdk@npm:@vector-im/matrix-bot-sdk@^0.6.6-element.1"`
-within the `matrix-protection-suite-for-matrix-bot-sdk` directory to ensure
-that that the local copy is using the same version as Draupnir.
-I don't understand why `yarn` will not respect overrides for linked
-dependencies.
+In either case, you should join our support room [#draupnir:matrix.org](https://matrix.to/#/#draupnir:matrix.org) :3
 
-#### VSCode
+Do not worry about making duplicates or alignment with project
+goals, the triage process is supposed to find that for you.
 
-You will also want to edit your `settings.json` to match something like
-this, so that you can debug into MPS while debugging Draupnir.
+### Fixing or implementing an existing issue
 
-```
-    "debug.javascript.terminalOptions": {
-        "runtimeArgs": ["--preserve-symlinks"],
-        "sourceMaps": true,
-        "outFiles": [
-            "${userHome}/experiments/draupnir/lib/**/*.js",
-            "${userHome}/experiments/draupnir/src/**/*.ts",
-            "${userHome}/experiments/draupnir/test/**/*.ts",
-            "${userHome}/experiments/matrix-protection-suite/dist/**/*.js",
-            "${userHome}/experiments/matrix-protection-suite/src/**/*.ts",
-            "${userHome}/experiments/matrix-protection-suite-for-matrix-bot-sdk/dist/**/*.js",
-            "${userHome}/experiments/matrix-protection-suite-for-matrix-bot-sdk/src/**/*.ts",
-          ]
-    }
-```
+If we have triaged the issue, even without writing our own context or
+clarifications, then the issue is likely ready to implement.
+
+You should write a small statement in the issue or a quick message to
+our support room about how you intend to resolve the issue before getting
+started.
+
+If you don't know how to get started or what to change, please ask!
+We'd love nothing more than to help you, or at the least, make
+our documentation and process better.
+
+## Where to start
+
+Join our room [#draupnir:matrix.org](https://matrix.to/#/#draupnir:matrix.org)!
+
+### How Draupnir works
+
+Checkout our [context document](./docs/context.md).
+
+### Code
+
+Checkout our [development guide](./docs/development.md).
 
-### mx-tester
-
-For integration testing, and spinning up a local synapse we use
-[mx-tester](https://github.com/matrix-org/mx-tester).
-While not required for basic changes, it is strongly recommended
-to use mx-tester or have the ability to spin up your own
-development Synapse to develop mjolnir interactively.
-
-To install `mx-tester` you will need the [rust toolchain](https://rustup.rs/)
-and Docker. You should refer to your linux distribution's documentation
-for installing both, and do not naively follow the instructions
-from rustup.rs without doing so first.
-Then you will be able to install `mx-tester` with `cargo install mx-tester`.
-Updating mx-tester can be done by installing `cargo install cargo-update`
-and using `cargo install-update mx-tester`, though you may skip
-this step until it is necessary to update `mx-tester`.
-
-#### Usage
-
-You can then start a local synapse using `mx-tester build`,
-followed by `mx-tester up`. You can then use `up`, `down` as many
-times as you like.
-If for some reason you need to get a clean Synapse database,
-you can just use `mx-tester down build`.
-
-### Debugging
-
-For debugging mx-tester it is recommended to use Visual Studio Code.
-If you open the project in visual studio code, press `F1`,
-type `Debug: JavaScript Debug Terminal`
-(see the [documentation](https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_javascript-debug-terminal)),
-and you should get a terminal from which node will always connect to
-Visual Studio Code.
-
-The following sections assume that a Synapse is running
-and `config/harness.yaml` has been configured to connect to it.
-If you are using `mx-tester` and you use `mx-tester up`, this will
-already be the case.
-
-#### Debugging and reproducing an issue
-
-If you need to debug an issue that is occurring through use in matrix,
-say the unban command has stopped working, you can launch
-mjolnir from the JavaScript Debug Terminal using `yarn test:manual`.
-This will launch mjolnir using the config found in `config/harness.yaml`.
-You can now open https://app.element.io, change the server to `localhost:8081`,
-and then create an account.
-From here you can join the room `#moderators:localhost:9999` (you will also be
-able to find it in the rooms directory) and interact with mjolnir.
-
-It is recommended to set breakpoints in the editor while interacting
-and switch the tab to "DEBUG CONSOLE" (within Visual Studio Code)
-to evaluate arbitrary expressions in the currently paused context (when
-a breakpoint has been hit).
-
-#### Debugging an integration test
-
-To debug the integration test suite from the JavaScript Debug Terminal,
-you can start them using `yarn test:integration`.
-However, more often than not there is a specific section of
-code you will be working on that has specific tests. Running
-the entire suite is therefore unnecessary.
-To run a specific test from the JavaScript Debug Terminal,
-you can use the script `yarn test:integration:single test/integration/banListTest.ts`,
-where `test/integration/banListTest.ts` is the name of the integration test you
-want to run.
-
-## Code style
-
-All Matrix projects have a well-defined code-style - and sometimes we've even
-got as far as documenting it... Mjolnir's code style is a relatively standard
-TypeScript project - run `yarn lint` to see how your code fairs.
-
-Before pushing new changes, ensure they don't produce linting errors.
-
-Please ensure your changes match the cosmetic style of the existing project,
-and **never** mix cosmetic and functional changes in the same commit, as it
-makes it horribly hard to review otherwise.
+### Issues & Triaging
+
+We don't have a specific guide for opening issues, just go ahead.
+
+You can read about our issue triaging process [here](./docs/triaging.md)
+
+### Documentation
+
+WIP, our documentation isn't great!
+
+If you know how we can improve that then let us know!
+
+Currently we just have markdown documents, but maybe we need
+something more complete? like a markdown book?
+
+Go ahead and edit anything.
+
+## Making Pull Requests
+
+The preferred and easiest way to contribute changes to Matrix is to fork the
+relevant project on github, and then [create a pull request](
+https://help.github.com/articles/using-pull-requests/) to ask us to pull
+your changes into our repo.
+
+We use Github Actions for continuous integration.
+If your change breaks the build, this will be shown in GitHub, so
+please keep an eye on the pull request for feedback.
 
 ## Sign off
 
diff --git a/README.md b/README.md
index 81ad66b4..a85564d4 100644
--- a/README.md
+++ b/README.md
@@ -127,63 +127,9 @@ However, if you are uncomfortable with this, please do not activate this feature
 Also, you should probably setup your `production.yaml` to ensure that the web
 server can only receive requests from your reverse proxy (e.g. `localhost`).
 
-## Development
+## Contributing & Opening Issues
 
-Draupnir is a TypeScript project that depends on the labour of a handful of
-developers, testers and users. The code base is in relatively good shape,
-and if you would like to contribute or gain an understanding of the workings
-of Draupnir, please read our [context document](./docs/context.md).
-
-Once you have done that, go ahead and read our [contributing document](./CONTRIBUTING.md)
-
-### Development and testing with mx-tester
-
-WARNING: mx-tester is currently work in progress, but it can still save you some time and is better than struggling with nothing.
-
-If you have docker installed you can quickly get setup with a development environment by using
-[mx-tester](https://github.com/matrix-org/mx-tester).
-
-To use mx-tester you will need to have rust installed. You can do that at [rustup](https://rustup.rs/) or [here](https://rust-lang.github.io/rustup/installation/other.html), you should probably also check your distro's documentation first to see if they have specific instructions for installing rust.
-
-Once rust is installed you can install mx-tester like so.
-
-```
-$ cargo install mx-tester
-```
-
-Once you have mx-tester installed you we will want to build a synapse image with synapse_antispam from the Draupnir project root.
-
-```
-$ mx-tester build
-```
-
-Then we can start a container that uses that image and the config in `mx-tester.yml`.
-
-```
-$ mx-tester up
-```
-
-Once you have called `mx-tester up` you can run the integration tests.
-```
-$ yarn test:integration
-```
-
-After calling `mx-tester up`, if we want to play with mojlnir locally we can run the following and then point a matrix client to http://localhost:9999.
-You should then be able to join the management room at `#moderators:localhost:9999`.
-
-```
-yarn test:manual
-```
-
-Once we are finished developing we can stop the synapse container.
-
-```
-mx-tester down
-```
-
-### Running integration tests
-
-The integration tests can be run with `yarn test:integration`.
-The config that the tests use is in `config/harness.yaml`
-and by default this is configured to work with the server specified in `mx-tester.yml`,
-but you can configure it however you like to run against your own setup.
+Draupnir wants to be yours as much as it is ours.
+Please see or [contributing document](./CONTRIBUTING.md), but do not
+worry too much about following the guidance to the letter. And
+keep that in mind throughout.
diff --git a/docs/code-style.md b/docs/code-style.md
new file mode 100644
index 00000000..3b13e601
--- /dev/null
+++ b/docs/code-style.md
@@ -0,0 +1,68 @@
+# Code style
+
+For now we don't have many objective code recommendations.
+Just try to stay consistent with the rest of the project,
+if that is alien to you, it's ok, just try. In the worst case we will
+clean things up for you.
+
+We give some general advice about style below.
+
+## Code style: optimisation
+
+One of the most important things a Draupnir developer should do is let
+to go of any tendencies that they may have towards micro optimisation.
+We want clear code so much more than anything else, optimisation should
+not be a concern at all when actually writing code.
+
+If you are somewhat unseasoned, you may find this somewhat puzzling.
+
+We believe the only way to effectively optimise is through design,
+and the use of a profiler. There are very few things more inefficient
+for programmers to work with than code written with overbearing
+concerns about performance. Using idioms and data structures
+appropriate for the programming language and problem being solved
+will always be enough. And I invite you to believe that,
+if micro optimisation is something you struggle with,
+then tell yourself that it does not matter.
+
+## Code style: scope
+
+You might notice that we try to keep the scope (which by "scope"
+here we really mean "how much stuff" is in a function/method)
+as small as possible. This is something you should stick to aswell.
+
+It really helps to introduce local functions if your method body starts
+getting too large. I can't really tell you how large too large is,
+but this quote captures the spirit well.
+
+> A friend of mine was once interviewing an engineer for a programming
+job and asked him a typical interview question: how do you know when a
+function or method is too big? Well, said the candidate, I don't like
+any method to be bigger than my head. You mean you can't keep all the
+details in your head? No, I mean I put my head up against my monitor,
+and the code shouldn't be bigger than my head.
+
+(Quote from Peter Seibel's [Practical Common Lisp](https://gigamonkeys.com/book/).
+
+## Code style: `const`
+
+Something you may notice is that we almost always try to use `const`.
+`const` should be the default when introducing a lexical variable
+(that's to say use `const` instead of `let` and `var`).
+
+You will very likely reach a point where you need to reassign a
+lexical variable. Don't stress too much about it, give it a try
+but if it's not practical then don't worry about using `let`,
+it's ok.
+
+A common hack that we use is an [IIFE](https://developer.mozilla.org/en-US/docs/Glossary/IIFE)
+if you have to, or just make local helper functions like so:
+
+### Why we do this?
+
+This improves the quality of code in numerous ways.
+First of all, we don't have to check if a variable is null, undefined,
+or some other default value because the variable is always initialized.
+Second of all it signals that we cannot reassign anything half
+way down the function body, and that takes off some mental load we'd
+otherwise have to  worry about.
diff --git a/docs/development-environment.md b/docs/development-environment.md
new file mode 100644
index 00000000..78c38d5c
--- /dev/null
+++ b/docs/development-environment.md
@@ -0,0 +1,162 @@
+# Developing Draupnir - tests, tools, and environment
+
+This document is a part of our [contributing documentation](../CONTRIBUTING.md)
+and describes how to setup a development environment that we to develop
+Draupnir. If you already have your own workflow for typescript projects,
+you should still read this document to spot any caveats that might
+require you to adapt for our recommendations.
+
+## matrix-protection-suite
+
+While not necessary, some changes will require you to make changes to the
+[matrix-protection-suite](https://github.com/Gnuxie/matrix-protection-suite)
+and the associated backend for the matrix-bot-sdk: [matrix-protection-suite-for-matrix-bot-sdk](https://github.com/Gnuxie/matrix-protection-suite-for-matrix-bot-sdk).
+
+You should clone these locally and then link them by using
+`yarn link` in each directory followed by `yarn link matirx-protection-suite matrix-protection-suite-for-matrix-bot-sdk` within Draupnir.
+
+You may also need to `yarn add --dev "matrix-bot-sdk@npm:@vector-im/matrix-bot-sdk@^0.6.6-element.1"`
+within the `matrix-protection-suite-for-matrix-bot-sdk` directory to ensure
+that that the local copy is using the same version as Draupnir.
+I don't understand why `yarn` will not respect overrides for linked
+dependencies.
+
+### VSCode
+
+You will also want to edit your `settings.json` to match something like
+this, so that you can debug into MPS while debugging Draupnir.
+
+```
+    "debug.javascript.terminalOptions": {
+        "runtimeArgs": ["--preserve-symlinks"],
+        "sourceMaps": true,
+        "outFiles": [
+            "${userHome}/experiments/draupnir/lib/**/*.js",
+            "${userHome}/experiments/draupnir/src/**/*.ts",
+            "${userHome}/experiments/draupnir/test/**/*.ts",
+            "${userHome}/experiments/matrix-protection-suite/dist/**/*.js",
+            "${userHome}/experiments/matrix-protection-suite/src/**/*.ts",
+            "${userHome}/experiments/matrix-protection-suite-for-matrix-bot-sdk/dist/**/*.js",
+            "${userHome}/experiments/matrix-protection-suite-for-matrix-bot-sdk/src/**/*.ts",
+          ]
+    }
+```
+
+## mx-tester
+
+WARNING: mx-tester is currently work in progress, but it can still save you some time and is better than struggling with nothing.
+
+For integration testing, and spinning up a local synapse we use
+[mx-tester](https://github.com/matrix-org/mx-tester).
+While not required for basic changes, it is strongly recommended
+to use mx-tester or have the ability to spin up your own
+development Synapse to develop mjolnir interactively.
+
+To install `mx-tester` you will need the [rust toolchain](https://rustup.rs/)
+and Docker. You should refer to your linux distribution's documentation
+for installing both, and do not naively follow the instructions
+from rustup.rs without doing so first.
+Then you will be able to install `mx-tester` with `cargo install mx-tester`.
+Updating mx-tester can be done by installing `cargo install cargo-update`
+and using `cargo install-update mx-tester`, though you may skip
+this step until it is necessary to update `mx-tester`.
+
+### Usage
+
+You can then start a local synapse using `mx-tester build`,
+followed by `mx-tester up`. You can then use `up`, `down` as many
+times as you like.
+If for some reason you need to get a clean Synapse database,
+you can just use `mx-tester down build`.
+
+### Development and testing with mx-tester
+
+If you have docker installed you can quickly get setup with a development environment by using
+[mx-tester](https://github.com/matrix-org/mx-tester).
+
+To use mx-tester you will need to have rust installed. You can do that at [rustup](https://rustup.rs/) or [here](https://rust-lang.github.io/rustup/installation/other.html), you should probably also check your distro's documentation first to see if they have specific instructions for installing rust.
+
+Once rust is installed you can install mx-tester like so.
+
+```
+$ cargo install mx-tester
+```
+
+Once you have mx-tester installed you we will want to build a synapse image with synapse_antispam from the Draupnir project root.
+
+```
+$ mx-tester build
+```
+
+Then we can start a container that uses that image and the config in `mx-tester.yml`.
+
+```
+$ mx-tester up
+```
+
+Once you have called `mx-tester up` you can run the integration tests.
+```
+$ yarn test:integration
+```
+
+After calling `mx-tester up`, if we want to play with mojlnir locally we can run the following and then point a matrix client to http://localhost:9999.
+You should then be able to join the management room at `#moderators:localhost:9999`.
+
+```
+yarn test:manual
+```
+
+Once we are finished developing we can stop the synapse container.
+
+```
+mx-tester down
+```
+
+## Debugging
+
+For debugging mx-tester it is recommended to use Visual Studio Code.
+If you open the project in visual studio code, press `F1`,
+type `Debug: JavaScript Debug Terminal`
+(see the [documentation](https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_javascript-debug-terminal)),
+and you should get a terminal from which node will always connect to
+Visual Studio Code.
+
+The following sections assume that a Synapse is running
+and `config/harness.yaml` has been configured to connect to it.
+If you are using `mx-tester` and you use `mx-tester up`, this will
+already be the case.
+
+### Debugging and reproducing an issue
+
+If you need to debug an issue that is occurring through use in matrix,
+say the unban command has stopped working, you can launch
+mjolnir from the JavaScript Debug Terminal using `yarn test:manual`.
+This will launch mjolnir using the config found in `config/harness.yaml`.
+You can now open https://app.element.io, change the server to `localhost:8081`,
+and then create an account.
+From here you can join the room `#moderators:localhost:9999` (you will also be
+able to find it in the rooms directory) and interact with mjolnir.
+
+It is recommended to set breakpoints in the editor while interacting
+and switch the tab to "DEBUG CONSOLE" (within Visual Studio Code)
+to evaluate arbitrary expressions in the currently paused context (when
+a breakpoint has been hit).
+
+### Running integration tests
+
+The integration tests can be run with `yarn test:integration`.
+The config that the tests use is in `config/harness.yaml`
+and by default this is configured to work with the server specified in `mx-tester.yml`,
+but you can configure it however you like to run against your own setup.
+
+### Debugging an integration test
+
+To debug the integration test suite from the JavaScript Debug Terminal,
+you can start them using `yarn test:integration`.
+However, more often than not there is a specific section of
+code you will be working on that has specific tests. Running
+the entire suite is therefore unnecessary.
+To run a specific test from the JavaScript Debug Terminal,
+you can use the script `yarn test:integration:single test/integration/banListTest.ts`,
+where `test/integration/banListTest.ts` is the name of the integration test you
+want to run.
diff --git a/docs/development.md b/docs/development.md
new file mode 100644
index 00000000..d19abe88
--- /dev/null
+++ b/docs/development.md
@@ -0,0 +1,16 @@
+# Developing Draupnir
+
+This document is a part of our [contributing documentation](../CONTRIBUTING.md),
+which you should take a read of if you didn't already.
+
+Draupnir is a TypeScript project that depends on the labour of a
+handful of developers, testers and users. The code base is in relatively
+good shape, and if you would like to contribute or gain an understanding
+of the workings of Draupnir, then please read our [context document](./docs/context.md).
+
+Then you should read our tests, tools and environment [document](./development-environment.md).
+As it goes into some details about how to setup your environment
+and debug code.
+
+Once you are happy to start writing code, you should glance at our
+[code style document](./code-style.md).
diff --git a/docs/triaging.md b/docs/triaging.md
new file mode 100644
index 00000000..052cfc60
--- /dev/null
+++ b/docs/triaging.md
@@ -0,0 +1,55 @@
+# Triaging issues
+
+The issue triaging process was developed in this [blog post](https://marewolf.me/posts/draupnir/2401.html#triaging)
+and we use [another post](https://lostgarden.home.blog/2008/05/20/improving-bug-triage-with-user-pain/)
+as our primary resouce.
+
+## The purpose of triaging
+
+For us, we use triaging primarily to create an order of issues that
+need attention first. It's very loose, and what should be worked on
+first will always remain subjective. However, this is a best effort
+of creating a list that can be easily looked at with less noise.
+
+
+## Labels
+### Likelihood
+
+Likelihood is just an estimation of the proportion of users that are
+likely to come across the issue, with a range from one to five.
+
+When the issue is within the context of a developer improvement,
+or something workflow related, you should consider the maximum
+value for the issue's likelihood to effect either users or developers.
+
+### Annoyance
+
+Annoyance is how annoying the issue is. This is a range from one to
+three and we avoid passive-aggressive labelling, for example labelling
+of an issue as "tolerable" is renown for frustrating issue reporters.
+When an issue is the most annoying, outrageous, the user is at risk of
+no longer wanting to use or engage with Draupnir altogether.
+This is independent of whether the user is technically able to continue.
+There is a reserved fourth level for something that blocks all development,
+for example a CI failure.
+Most issues are expected to sit around the second level, aggravating.
+
+### Impact
+
+Finally there is a categorisation of the issue type, which we call impact.
+The reason we call this impact and not type, is because this seems to
+be a shortcoming in the original model. They have a linear score for
+the issue type label, and put documentation and visual issues at the
+lower end. From what I can tell, the intention of the scoring is to
+represent how the issue relates to workflow. The highest score, crash,
+means that work can't continue or there's other consequences such as
+data loss. So by calling this label impact instead of type, we are
+explicitly saying that its purpose is to highlight issues that hinder
+people's ability to use or continue to use Draupnir. This includes
+documentation issues that would prevent them from setting up Draupnir
+or describing how they can use a feature. If a documentation issue
+means that a new user can't start or use Draupnir, then this will
+still be tier six, which is named crash, rather than tier two or below.
+This is extremely important because categorising issues naively by type
+and then ranking them (as Tailscale and the linked blog posts appear to)
+would make critical documentation issues seem less important at a glance.