From 9e19c6aab4b5a99039f2ddc7d3120dd3b26c274b Mon Sep 17 00:00:00 2001 From: David Teller Date: Wed, 17 Feb 2021 17:23:57 +0100 Subject: [PATCH] Reorganize CONTRIBUTING.md documentation. (#9281) --- CONTRIBUTING.md | 271 ++++++++++++++++++++++++++++++------------- changelog.d/9281.doc | 1 + 2 files changed, 190 insertions(+), 82 deletions(-) create mode 100644 changelog.d/9281.doc diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1d7bb8f969..b6a70f7ffe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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) +- [3. Get the source.](#3-get-the-source) +- [4. Install the dependencies](#4-install-the-dependencies) + * [Under Unix (macOS, Linux, BSD, ...)](#under-unix-macos-linux-bsd-) + * [Under Windows](#under-windows) +- [5. Get in touch.](#5-get-in-touch) +- [6. Pick an issue.](#6-pick-an-issue) +- [7. Turn coffee and documentation into code and documentation!](#7-turn-coffee-and-documentation-into-code-and-documentation) +- [8. Test, test, test!](#8-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) +- [9. Submit your patch.](#9-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) + + [Debian changelog](#debian-changelog) + * [Sign off](#sign-off) +- [10. Turn feedback into better code.](#10-turn-feedback-into-better-code) +- [11. Find a new issue.](#11-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 @@ -9,70 +36,179 @@ 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/). + + +# 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]( +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 git@github.com:YOUR_GITHUB_USER_NAME/synapse.git +git checkout develop +``` - * Please follow the [code style requirements](#code-style). +If you need help getting started with git, this is beyond the scope of the document, but you +can find many good git tutorials on the web. - * Please include a [changelog entry](#changelog) with each PR. +# 4. Install the dependencies - * Please [sign off](#sign-off) your contribution. +## Under Unix (macOS, Linux, BSD, ...) - * 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. +Once you have installed Python 3 and added the source, please open a terminal and +setup a *virtualenv*, as follows: - * If you need to [update your PR](#updating-your-pull-request), just add new - commits to your branch rather than rebasing. +```sh +cd path/where/you/have/cloned/the/repository +python3 -m venv ./env +source ./env/bin/activate +pip install -e ".[all,lint,mypy,test]" +pip install tox +``` -## Code style +This will install the developer dependencies for the project. + +## Under Windows + +TBD + + +# 5. Get in touch. + +Join our developer community on Matrix: #synapse-dev:matrix.org ! + + +# 6. Pick an issue. + +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. + + +# 7. 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. -``` -# Install the dependencies -pip install -e ".[lint,mypy]" +If you add new files added to either of these folders, please use [GitHub-Flavoured +Markdown](https://guides.github.com/features/mastering-markdown/). -# Run the linter script +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. + + +# 8. Test, test, test! + + +While you're developing and before submitting a patch, you'll +want to test your code. + +## 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 this script *will modify your files* to fix styling errors. +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. +## Run the unit tests. -Before pushing new changes, ensure they don't produce linting errors. Commit any -files that were corrected. +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 +trial tests +``` + +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 +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 are a more comprehensive suite of tests. They +run a full version of Synapse, including your changes, to check if +anything was broken. They are slower than the unit tests but will +typically catch more errors. + +The following command will let you run the integration test with the most common +configuration: + +```sh +$ docker run --rm -it -v /path/where/you/have/cloned/the/repository\:/src:ro -v /path/to/where/you/want/logs\:/logs matrixdotorg/sytest-synapse:py37 +``` + +This configuration should generally cover your needs. For more details about other configurations, see [documentation in the SyTest repo](https://github.com/matrix-org/sytest/blob/develop/docker/README.md). + + +# 9. Submit your patch. + +Once you're happy with your patch, it's time to prepare a Pull Request. + +To prepare a Pull Request, please: + +1. verify that [all the tests pass](#test-test-test), including the coding style; +2. [sign off](#sign-off) your contribution; +3. `git push` your commit to your fork of Synapse; +4. on GitHub, [create the Pull Request](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request); +5. add a [changelog entry](#changelog) and push it to your Pull Request; +6. for most contributors, that's all - however, if you are a member of the organization `matrix-org`, on GitHub, please request a review from `matrix.org / Synapse Core`. -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. ## Changelog @@ -156,24 +292,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 @@ -240,47 +358,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. +# 10. Turn feedback into better code. -To run unit tests in a local development environment, you can use: +Once the Pull Request is opened, you will see a few things: -- ``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. +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. -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. +From this point, you should: -## Updating your pull request +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 and let us know if it is ready for a developer to review again. +3. Create a new commit with the changes. + - Please do NOT overwrite the history. New commits make the reviewer's life easier. + - Push this commits to your Pull Request. +4. Back to 1. -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. +Once both the CI and the developers are happy, the patch will be merged into Synapse and released shortly! -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. +# 11. Find a new issue. -## Notes for maintainers on merging PRs etc +By now, you know the drill! + +# 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 diff --git a/changelog.d/9281.doc b/changelog.d/9281.doc new file mode 100644 index 0000000000..4dea375f80 --- /dev/null +++ b/changelog.d/9281.doc @@ -0,0 +1 @@ +Reorganizing CHANGELOG.md. \ No newline at end of file