general updates to contributing
Some general tidying and clarification, to start us off.pull/7496/head
parent
204664d1ad
commit
23c5d854ce
148
CONTRIBUTING.md
148
CONTRIBUTING.md
|
@ -1,19 +1,20 @@
|
||||||
# Contributing code to Matrix
|
# Contributing code to Synapse
|
||||||
|
|
||||||
Everyone is welcome to contribute code to Matrix
|
Everyone is welcome to contribute code to [matrix.org
|
||||||
(https://github.com/matrix-org), provided that they are willing to license
|
projects](https://github.com/matrix-org), provided that they are willing to
|
||||||
their contributions under the same license as the project itself. We follow a
|
license their contributions under the same license as the project itself. We
|
||||||
simple 'inbound=outbound' model for contributions: the act of submitting an
|
follow a simple 'inbound=outbound' model for contributions: the act of
|
||||||
'inbound' contribution means that the contributor agrees to license the code
|
submitting an 'inbound' contribution means that the contributor agrees to
|
||||||
under the same terms as the project's overall 'outbound' license - in our
|
license the code under the same terms as the project's overall 'outbound'
|
||||||
case, this is almost always Apache Software License v2 (see [LICENSE](LICENSE)).
|
license - in our case, this is almost always Apache Software License v2 (see
|
||||||
|
[LICENSE](LICENSE)).
|
||||||
|
|
||||||
## How to contribute
|
## How to contribute
|
||||||
|
|
||||||
The preferred and easiest way to contribute changes to Matrix is to fork the
|
The preferred and easiest way to contribute changes is to fork the relevant
|
||||||
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
|
https://help.github.com/articles/using-pull-requests/) to ask us to pull your
|
||||||
your changes into our repo.
|
changes into our repo.
|
||||||
|
|
||||||
**The single biggest thing you need to know is: please base your changes on
|
**The single biggest thing you need to know is: please base your changes on
|
||||||
the develop branch - *not* master.**
|
the develop branch - *not* master.**
|
||||||
|
@ -28,35 +29,29 @@ use github's pull request workflow to review the contribution, and either ask
|
||||||
you to make any refinements needed or merge it and make them ourselves. The
|
you to make any refinements needed or merge it and make them ourselves. The
|
||||||
changes will then land on master when we next do a release.
|
changes will then land on master when we next do a release.
|
||||||
|
|
||||||
We use [Buildkite](https://buildkite.com/matrix-dot-org/synapse) for continuous
|
Some other things you will need to know when contributing to Synapse:
|
||||||
integration. If your change breaks the build, this will be shown in GitHub, so
|
|
||||||
please keep an eye on the pull request for feedback.
|
|
||||||
|
|
||||||
To run unit tests in a local development environment, you can use:
|
* Please follow the [code style requirements](#code-style).
|
||||||
|
|
||||||
- ``tox -e py35`` (requires tox to be installed by ``pip install tox``)
|
* Please include a [changelog entry](#changelog) with each PR.
|
||||||
for SQLite-backed Synapse on Python 3.5.
|
|
||||||
- ``tox -e py36`` for SQLite-backed Synapse on Python 3.6.
|
* Please [sign off](#sign-off) your contribution.
|
||||||
- ``tox -e py36-postgres`` for PostgreSQL-backed Synapse on Python 3.6
|
|
||||||
(requires a running local PostgreSQL with access to create databases).
|
* Please keep an eye on the pull request for feedback from the [continuous
|
||||||
- ``./test_postgresql.sh`` for PostgreSQL-backed Synapse on Python 3.5
|
integration system](#continuous-integration-and-testing) and try to fix any
|
||||||
(requires Docker). Entirely self-contained, recommended if you don't want to
|
errors that come up.
|
||||||
set up PostgreSQL yourself.
|
|
||||||
|
|
||||||
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.
|
|
||||||
|
|
||||||
## Code style
|
## Code style
|
||||||
|
|
||||||
All Matrix projects have a well-defined code-style - and sometimes we've even
|
Synapse's code style is documented [here](docs/code_style.md). Please follow
|
||||||
got as far as documenting it... For instance, synapse's code style doc lives
|
it, including the conventions for the [sample configuration
|
||||||
[here](docs/code_style.md).
|
file](docs/code_style.md#configuration-file-format).
|
||||||
|
|
||||||
To facilitate meeting these criteria you can run `scripts-dev/lint.sh`
|
Many of the conventions are enforced by scripts which are run as part of the
|
||||||
locally. Since this runs the tools listed in the above document, you'll need
|
[continuous integration system](#continuous-integration-and-testing). To help
|
||||||
python 3.6 and to install each tool:
|
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:
|
||||||
|
|
||||||
```
|
```
|
||||||
# Install the dependencies
|
# Install the dependencies
|
||||||
|
@ -67,9 +62,11 @@ pip install -U black flake8 flake8-comprehensions isort
|
||||||
```
|
```
|
||||||
|
|
||||||
**Note that the script does not just test/check, but also reformats code, so you
|
**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**. By default this script
|
may wish to ensure any new code is committed first**.
|
||||||
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:
|
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:
|
||||||
|
|
||||||
```
|
```
|
||||||
./scripts-dev/lint.sh path/to/file1.py path/to/file2.py path/to/folder
|
./scripts-dev/lint.sh path/to/file1.py path/to/file2.py path/to/folder
|
||||||
|
@ -82,7 +79,6 @@ 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
|
and **never** mix cosmetic and functional changes in the same commit, as it
|
||||||
makes it horribly hard to review otherwise.
|
makes it horribly hard to review otherwise.
|
||||||
|
|
||||||
|
|
||||||
## Changelog
|
## Changelog
|
||||||
|
|
||||||
All changes, even minor ones, need a corresponding changelog / newsfragment
|
All changes, even minor ones, need a corresponding changelog / newsfragment
|
||||||
|
@ -98,24 +94,54 @@ in the format of `PRnumber.type`. The type can be one of the following:
|
||||||
* `removal` (also used for deprecations)
|
* `removal` (also used for deprecations)
|
||||||
* `misc` (for internal-only changes)
|
* `misc` (for internal-only changes)
|
||||||
|
|
||||||
The content of the file is your changelog entry, which should be a short
|
This file will become part of our [changelog](
|
||||||
description of your change in the same style as the rest of our [changelog](
|
https://github.com/matrix-org/synapse/blob/master/CHANGES.md) at the next
|
||||||
https://github.com/matrix-org/synapse/blob/master/CHANGES.md). The file can
|
release, so the content of the file should be a short description of your
|
||||||
contain Markdown formatting, and should end with a full stop (.) or an
|
change in the same style as the rest of the changelog. The file can contain Markdown
|
||||||
exclamation mark (!) for consistency.
|
formatting, and should end with a full stop (.) or an exclamation mark (!) for
|
||||||
|
consistency.
|
||||||
|
|
||||||
Adding credits to the changelog is encouraged, we value your
|
Adding credits to the changelog is encouraged, we value your
|
||||||
contributions and would like to have you shouted out in the release notes!
|
contributions and would like to have you shouted out in the release notes!
|
||||||
|
|
||||||
For example, a fix in PR #1234 would have its changelog entry in
|
For example, a fix in PR #1234 would have its changelog entry in
|
||||||
`changelog.d/1234.bugfix`, and contain content like "The security levels of
|
`changelog.d/1234.bugfix`, and contain content like:
|
||||||
Florbs are now validated when received over federation. Contributed by Jane
|
|
||||||
Matrix.".
|
|
||||||
|
|
||||||
## Debian changelog
|
> The security levels of Florbs are now validated when received over
|
||||||
|
> via the `/federation/florb` endpoint. Contributed by Jane Matrix.
|
||||||
|
|
||||||
|
If there are multiple pull requests involved in a single bugfix/feature/etc,
|
||||||
|
then the content for each `changelog.d` file should be the same. Towncrier will
|
||||||
|
merge the matching files together into a single changelog entry when we come to
|
||||||
|
release.
|
||||||
|
|
||||||
|
### How do I know what to call the changelog file before I create the PR?
|
||||||
|
|
||||||
|
Obviously, you don't know if you should call your newsfile
|
||||||
|
`1234.bugfix` or `5678.bugfix` until you create the PR, which leads to a
|
||||||
|
chicken-and-egg problem.
|
||||||
|
|
||||||
|
There are two options for solving this:
|
||||||
|
|
||||||
|
1. Open the PR without a changelog file, see what number you got, and *then*
|
||||||
|
add the changelog file to your branch, or:
|
||||||
|
|
||||||
|
1. look at the [list of all
|
||||||
|
issues/PRs](https://github.com/matrix-org/synapse/issues?q=), add one to the
|
||||||
|
highest number you see, and quickly open the PR before somebody else claims
|
||||||
|
your number.
|
||||||
|
|
||||||
|
[This
|
||||||
|
script](https://github.com/richvdh/scripts/blob/master/next_github_number.sh)
|
||||||
|
might be helpful if you find yourself doing this a lot.
|
||||||
|
|
||||||
|
Sorry, we know it's a bit fiddly, but it's *really* helpful for us when we come
|
||||||
|
to put together a release!
|
||||||
|
|
||||||
|
### Debian changelog
|
||||||
|
|
||||||
Changes which affect the debian packaging files (in `debian`) are an
|
Changes which affect the debian packaging files (in `debian`) are an
|
||||||
exception.
|
exception to the rule that all changes require a `changelog.d` file.
|
||||||
|
|
||||||
In this case, you will need to add an entry to the debian changelog for the
|
In this case, you will need to add an entry to the debian changelog for the
|
||||||
next release. For this, run the following command:
|
next release. For this, run the following command:
|
||||||
|
@ -200,6 +226,30 @@ 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
|
flag to `git commit`, which uses the name and email set in your
|
||||||
`user.name` and `user.email` git configs.
|
`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.
|
||||||
|
|
||||||
|
To run unit tests in a local development environment, you can use:
|
||||||
|
|
||||||
|
- ``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.
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
## Merge Strategy
|
## Merge Strategy
|
||||||
|
|
||||||
We use the commit history of develop/master extensively to identify
|
We use the commit history of develop/master extensively to identify
|
||||||
|
@ -210,7 +260,7 @@ changes into develop. For small changes this means there is no need to rebase
|
||||||
to clean up your PR before merging. Larger changes with an organised set of
|
to clean up your PR before merging. Larger changes with an organised set of
|
||||||
commits may be merged as-is, if the history is judged to be useful.
|
commits may be merged as-is, if the history is judged to be useful.
|
||||||
|
|
||||||
This use of squash-merging will mean PRs built on each other will be hard to
|
This use of squash-merging will mean PRs built on each other will be hard to
|
||||||
merge. We suggest avoiding these where possible, and if required, ensuring
|
merge. We suggest avoiding these where possible, and if required, ensuring
|
||||||
each PR has a tidy set of commits to ease merging.
|
each PR has a tidy set of commits to ease merging.
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue