Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributing guidelines for new changelog generation #1787

Merged
merged 11 commits into from
Jul 15, 2021
4 changes: 4 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
<!-- Please read https://github.com/matrix-org/matrix-js-sdk/blob/develop/CONTRIBUTING.rst before submitting your pull request -->

<!-- Include a Sign-Off as described in https://github.com/matrix-org/matrix-js-sdk/blob/develop/CONTRIBUTING.rst#sign-off -->

<!-- To specify text for the changelog entry (otherwise the PR title will be used):
Notes:
-->
83 changes: 71 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,77 @@ The preferred and easiest way to contribute changes to the project is to fork
it on github, and then create a pull request to ask us to pull your changes
into our repo (https://help.github.com/articles/using-pull-requests/)

**The single biggest thing you need to know is: please base your changes on
the develop branch - /not/ master.**

We use the master branch to track the most recent release, so that folks who
blindly clone the repo and automatically check out master get something that
works. Develop is the unstable branch where all the development actually
happens: the workflow is that contributors should fork the develop branch to
make a 'feature' branch for a particular contribution, and then make a pull
request to merge this back into the matrix.org 'official' develop branch. We
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
changes will then land on master when we next do a release.
We 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.

Things that should go into your PR description:
* A changelog entry in the `Notes` section (see below)
* References to any bugs fixed by the change (in Github's `Fixes` notation)
dbkr marked this conversation as resolved.
Show resolved Hide resolved
* Notes for the reviewer that might help them to understand why the change is
necessary or how they might better review it.

Things that should *not* go into your PR description:
* Any information on how the code works or why you chose to do it the way
you did. If this isn't obvious from your code, you haven't written enough
comments.

We rely on information in pull request to populate the information that goes
into the changelogs our users see, both for the js-sdk itself and also for some
dbkr marked this conversation as resolved.
Show resolved Hide resolved
projects based on it. This is picked up from both labels on the pull request and
the `Notes: ` annotation in the description. By default, the PR title will be
dbkr marked this conversation as resolved.
Show resolved Hide resolved
used for the changelog entry, but you can specify more options, as follows.

To add a longer, more detailed description of the change for the changelog:


*Fix llama herding bug*

```
Notes: Fix a bug (https://github.com/matrix-org/notaproject/issues/123) where the 'Herd' button would not herd more than 8 Llamas if the moon was in the waxing gibbous phase
```

For some PRs, it's not useful to have an entry in the user-facing changelog (this is
the default for PRs labelled with `pr-internal`):
dbkr marked this conversation as resolved.
Show resolved Hide resolved

*Remove outdated comment from `Ungulates.ts`*
```
Notes: none
```

Sometimes, you're fixing a bug in a downstream project, in which case you want
an entry in that project's changelog. You can do that too:

*Fix another herding bug*
```
Notes: Fix a bug where the `herd()` function would only work on Tuesdays
other-project notes: Fix a bug where the 'Herd' button only worked on Tuesdays
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I understand correctly that the other-project should get replaced with a project name? If so, could it perhaps be made more clear (<other-project> or similar), it wasn't clear to me at the start

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah - maybe a real-life example will be clearer: updated.

```

Projects that you can specify here are:
* matrix-react-sdk
* element-web
* element-desktop

If your PR introduces a breaking change, use the `Notes` section in the same
way, additionally adding the `pr-breaking` label (see below). There's no need
dbkr marked this conversation as resolved.
Show resolved Hide resolved
to specify in the notes that it's a breaking change - this will be added
automatically based on the label - but remember to tell the developer how to
migrate:

*Remove legacy class*

```
Notes: Remove legacy `Camelopard` class. `Giraffe` should be used instead.
```

Other metadata can be added using labels.
* `pr-breaking`: A breaking change - adding this label will mean the change causes a *major* version bump.
* `pr-feature`: A new feature - adding this label will mean the change causes a *minor* version bump.
* `pr-bugfix`: A bugfix (in either code or docs).
* `pr-internal`: No user-facing changes, eg. code comments, CI fixes, refactors or tests. Won't have a changelog entry unless you specify one.
dbkr marked this conversation as resolved.
Show resolved Hide resolved

If you don't have permission to add labels, your PR reviewer(s) can work with you
to add them: ask in the PR description or comments.

We use continuous integration, and all pull requests get automatically tested:
if your change breaks the build, then the PR will show that there are failed
Expand Down