Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Reorganize CONTRIBUTING.md #9281

Merged
merged 8 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 196 additions & 81 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,31 @@
# Contributing code to Synapse
Welcome to Synapse

This document aims to get you started with contributing to this repo!

- [1. Who can contribute to Synapse?](#1-who-can-contribute-to-synapse-)
- [2. What do I need?](#2-what-do-i-need-)
clokep marked this conversation as resolved.
Show resolved Hide resolved
* [Under Unix (macOS, Linux, BSD, ...)](#under-unix--macos--linux--bsd---)
clokep marked this conversation as resolved.
Show resolved Hide resolved
* [Under Windows](#under-windows)
- [3. Get the source.](#3-get-the-source)
- [4. Get in touch.](#4-get-in-touch)
- [5. Pick an issue.](#5-pick-an-issue)
- [6. Turn coffee and documentation into code and documentation!](#6-turn-coffee-and-documentation-into-code-and-documentation-)
- [7. Test, test, test!](#7-test--test--test-)
* [Run the linters.](#run-the-linters)
* [Run the unit tests.](#run-the-unit-tests)
* [Run the integration tests.](#run-the-integration-tests)
- [8. Submit your patch.](#8-submit-your-patch)
* [Changelog](#changelog)
+ [How do I know what to call the changelog file before I create the PR?](#how-do-i-know-what-to-call-the-changelog-file-before-i-create-the-pr-)
clokep marked this conversation as resolved.
Show resolved Hide resolved
+ [Debian changelog](#debian-changelog)
* [Sign off](#sign-off)
- [9. Turn feedback into better code.](#9-turn-feedback-into-better-code)
- [10. Find a new issue.](#10-find-a-new-issue)
- [Notes for maintainers on merging PRs etc](#notes-for-maintainers-on-merging-prs-etc)
- [Conclusion](#conclusion)


# 1. Who can contribute to Synapse?

Everyone is welcome to contribute code to [matrix.org
projects](https://github.com/matrix-org), provided that they are willing to
Expand All @@ -9,70 +36,187 @@ license the code under the same terms as the project's overall 'outbound'
license - in our case, this is almost always Apache Software License v2 (see
[LICENSE](LICENSE)).

## How to contribute
# 2. What do I need?

The code of Synapse is written in Python 3. To do pretty much anything, you'll need [a recent version of Python 3](https://wiki.python.org/moin/BeginnersGuide/Download).

The source code of Synapse is hosted on Github. You will also need [a recent version of git](https://github.com/git-guides/install-git).

For some tests, you will need [a recent version of Docker](https://docs.docker.com/get-docker/).

## Under Unix (macOS, Linux, BSD, ...)

Once you have installed Python 3, please open a Terminal and run:
Yoric marked this conversation as resolved.
Show resolved Hide resolved

```sh
mkdir -p ~/synapse
python3 -m venv ./env
source ./env/bin/activate
Yoric marked this conversation as resolved.
Show resolved Hide resolved
pip install -e ".[all,lint,mypy,test]"
pip install tox
```

This will install the developer dependencies for the project.

## Under Windows

TBD


# 3. Get the source.

The preferred and easiest way to contribute changes 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.

Some other points to follow:
Please base your changes on the `develop` branch.

* Please base your changes on the `develop` branch.
```sh
git clone [email protected]:YOUR_GITHUB_USER_NAME/synapse.git
```
richvdh marked this conversation as resolved.
Show resolved Hide resolved

* Please follow the [code style requirements](#code-style).
# 4. Get in touch.

* Please include a [changelog entry](#changelog) with each PR.
Join our developer community on Matrix: #synapse-dev:matrix.org !

* Please [sign off](#sign-off) your contribution.

* Please keep an eye on the pull request for feedback from the [continuous
integration system](#continuous-integration-and-testing) and try to fix any
errors that come up.
# 5. Pick an issue.

* If you need to [update your PR](#updating-your-pull-request), just add new
commits to your branch rather than rebasing.
Fix your favorite problem or perhaps find a [Good First Issue](https://github.com/matrix-org/synapse/issues?q=is%3Aopen+is%3Aissue+label%3A%22Good+First+Issue%22)
to work on.

## Code style

# 6. Turn coffee and documentation into code and documentation!

Synapse's code style is documented [here](docs/code_style.md). Please follow
it, including the conventions for the [sample configuration
file](docs/code_style.md#configuration-file-format).

Many of the conventions are enforced by scripts which are run as part of the
[continuous integration system](#continuous-integration-and-testing). To help
check if you have followed the code style, you can run `scripts-dev/lint.sh`
locally. You'll need python 3.6 or later, and to install a number of tools:
There is a growing amount of documentation located in the [docs](docs)
directory. This documentation is intended primarily for sysadmins running their
own Synapse instance, as well as developers interacting externally with
Synapse. [docs/dev](docs/dev) exists primarily to house documentation for
Synapse developers. [docs/admin_api](docs/admin_api) houses documentation
regarding Synapse's Admin API, which is used mostly by sysadmins and external
service developers.

If you add new files added to either of these folders, please use [Github-Flavoured
Markdown](https://guides.github.com/features/mastering-markdown/).

```
# Install the dependencies
pip install -e ".[lint,mypy]"
Some documentation also exists in [Synapse's Github
Wiki](https://github.com/matrix-org/synapse/wiki), although this is primarily
contributed to by community authors.


# 7. Test, test, test!
<a name="test-test-test"></a>

While you're developing and before submitting a patch, you'll
want to test your code.

# Run the linter script
## Run the linters.

The linters look at your code and do two things:

- ensure that your code follows the coding style adopted by the project;
- catch a number of errors in your code.

They're pretty fast, don't hesitate!

```sh
source ./env/bin/activate
./scripts-dev/lint.sh
```

**Note that the script does not just test/check, but also reformats code, so you
may wish to ensure any new code is committed first**.
Note that the linters *will modify your files* to fix styling errors.
Yoric marked this conversation as resolved.
Show resolved Hide resolved
Make sure that you have saved all your files.

By default, this script checks all files and can take some time; if you alter
only certain files, you might wish to specify paths as arguments to reduce the
run-time:
If you wish to restrict the linters to only the files changed since the last commit
(much faster!), you can instead run:

```sh
source ./env/bin/activate
./scripts-dev/lint.sh -d
```

Or if you know exactly which files you wish to lint, you can instead run:

```sh
source ./env/bin/activate
./scripts-dev/lint.sh path/to/file1.py path/to/file2.py path/to/folder
```

You can also provide the `-d` option, which will lint the files that have been
changed since the last git commit. This will often be significantly faster than
linting the whole codebase.
Yoric marked this conversation as resolved.
Show resolved Hide resolved
## Run the unit tests.

The unit tests run parts of Synapse, including your changes, to see if anything
was broken. They are slower than the linters but will typically catch more errors.

```sh
source ./env/bin/activate
python -m twisted.trial tests
Yoric marked this conversation as resolved.
Show resolved Hide resolved
```

If you wish to only run *some* unit tests, you may specify
another module instead of `tests` - or a test class or a method:

```sh
source ./env/bin/activate
python -m twisted.trial tests.rest.admin.test_room tests.handlers.test_admin.ExfiltrateData.test_invite
```

If your tests fail, you may wish to look at the logs:

```sh
less _trial_temp/test.log
```

## Run the integration tests.

The integration tests run a full Synapse, including your changes, to
see if anything was broken. They are slower than the unit tests but will
typically catch more errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some misunderstanding here. tox runs the same set of tests as trial. I'm not entirely sure I'd describe those tests as integration tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, is there any way to run integration tests?

If not, I suppose that I should entirely remove this subsection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration tests are run via sytest, we should probably point to the sytest repo that has it's own readme and not go into detail with it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integration tests are run via sytest, we should probably point to the sytest repo that has it's own readme and not go into detail with it here.

I believe that it would be better if at least an example was in this document.

Now, I just need to understand that README.md :)

To run the entire test suite using Sqlite3, use the following command:

```sh
source ./env/bin/activate
tox -e py36
```

If you wish to only run *some* integration tests, you may
similarly specify a module, e.g.:

```sh
source ./env/bin/activate
tox -e py36 -- test.rest.admin.test_room
```



Or, to run the test suite using postgres, use the following command. You will need [Docker](https://docs.docker.com/get-docker/) installed.

```sh
source ./env/bin/activate
./test_postgresql.sh
```


# 8. Submit your patch.

Before pushing new changes, ensure they don't produce linting errors. Commit any
files that were corrected.
Once you're happy with your patch, it's time to prepare a Pull Request.

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.
To prepare a Pull Request, please:

1. verify that [all the tests pass](#test-test-test), including the coding style;
2. add a [changelog entry](#changelog);
3. [sign off](#sign-off) your contribution;
4. Push code to commits to your fork of Synapse:
```sh
git push origin develop
```
Yoric marked this conversation as resolved.
Show resolved Hide resolved
5. on Github, [create the Pull Request](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request);
6. on Github, request review from `matrix.org / Synapse Core`.

## Changelog

Expand Down Expand Up @@ -156,24 +300,6 @@ directory, you will need both a regular newsfragment *and* an entry in the
debian changelog. (Though typically such changes should be submitted as two
separate pull requests.)

## Documentation

There is a growing amount of documentation located in the [docs](docs)
directory. This documentation is intended primarily for sysadmins running their
own Synapse instance, as well as developers interacting externally with
Synapse. [docs/dev](docs/dev) exists primarily to house documentation for
Synapse developers. [docs/admin_api](docs/admin_api) houses documentation
regarding Synapse's Admin API, which is used mostly by sysadmins and external
service developers.

New files added to both folders should be written in [Github-Flavoured
Markdown](https://guides.github.com/features/mastering-markdown/), and attempts
should be made to migrate existing documents to markdown where possible.

Some documentation also exists in [Synapse's Github
Wiki](https://github.com/matrix-org/synapse/wiki), although this is primarily
contributed to by community authors.

## Sign off

In order to have a concrete record that your contribution is intentional
Expand Down Expand Up @@ -240,47 +366,36 @@ Git allows you to add this signoff automatically when using the `-s`
flag to `git commit`, which uses the name and email set in your
`user.name` and `user.email` git configs.

## Continuous integration and testing

[Buildkite](https://buildkite.com/matrix-dot-org/synapse) will automatically
run a series of checks and tests against any PR which is opened against the
project; if your change breaks the build, this will be shown in GitHub, with
links to the build results. If your build fails, please try to fix the errors
and update your branch.
# 9. Turn feedback into better code.

Once the Pull Request is opened, you will see a few things:

To run unit tests in a local development environment, you can use:
1. our automated CI (Continuous Integration) pipeline will run (again) the linters, the unit tests, the integration tests and more;
2. one or more of the developers will take a look at your Pull Request and offer feedback.

- ``tox -e py35`` (requires tox to be installed by ``pip install tox``)
for SQLite-backed Synapse on Python 3.5.
- ``tox -e py36`` for SQLite-backed Synapse on Python 3.6.
- ``tox -e py36-postgres`` for PostgreSQL-backed Synapse on Python 3.6
(requires a running local PostgreSQL with access to create databases).
- ``./test_postgresql.sh`` for PostgreSQL-backed Synapse on Python 3.5
(requires Docker). Entirely self-contained, recommended if you don't want to
set up PostgreSQL yourself.
From this point, you should:

Docker images are available for running the integration tests (SyTest) locally,
see the [documentation in the SyTest repo](
https://github.com/matrix-org/sytest/blob/develop/docker/README.md) for more
information.
1. Look at the results of the CI pipeline.
- If there is any error, fix the error.
2. If a developer has requested changes, make these changes.
clokep marked this conversation as resolved.
Show resolved Hide resolved
3. Create a new commit with the changes.
- Please [do NOT overwrite the history](docs/dev/git.md#on-keeping-the-commit-history-clean). New commits make the reviewer's life easier.
Yoric marked this conversation as resolved.
Show resolved Hide resolved
- Push this commits to your Pull Request.
4. Back to 1.

## Updating your pull request
Once both the CI and the developers are happy, the patch will be merged into Synapse and released shortly!

If you decide to make changes to your pull request - perhaps to address issues
raised in a review, or to fix problems highlighted by [continuous
integration](#continuous-integration-and-testing) - just add new commits to your
branch, and push to GitHub. The pull request will automatically be updated.
# 10. Find a new issue.

Please **avoid** rebasing your branch, especially once the PR has been
reviewed: doing so makes it very difficult for a reviewer to see what has
changed since a previous review.
By now, you know the drill!

## Notes for maintainers on merging PRs etc
# Notes for maintainers on merging PRs etc

There are some notes for those with commit access to the project on how we
manage git [here](docs/dev/git.md).

## Conclusion
# Conclusion

That's it! Matrix is a very open and collaborative project as you might expect
given our obsession with open communication. If we're going to successfully
Expand Down
1 change: 1 addition & 0 deletions changelog.d/9281.doc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reorganizing CHANGELOG.md.