Skip to content

Commit

Permalink
contributing documentation overhaul. (#344)
Browse files Browse the repository at this point in the history 
* contributing documentation overhaul.

* hm

* declutter development stuff from readme.

* hmm

* broken link
  • Loading branch information
@Gnuxie
Gnuxie authored Apr 15, 2024
1 parent 97482aa commit 6d46229
Show file tree
Hide file tree
Showing 6 changed files with 382 additions and 180 deletions.
197 changes: 76 additions & 121 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
@@ -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

Expand Down
64 changes: 5 additions & 59 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
68 changes: 68 additions & 0 deletions docs/code-style.md
View file
Original file line number Diff line number Diff line change
@@ -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.
Loading

0 comments on commit 6d46229

Please sign in to comment.