From b538c06fcb60d049e8395f5ab490ef095c35de6f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 19:45:42 +0100 Subject: [PATCH 01/10] Update contributing guidelines to for new changelog stuff --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++ CONTRIBUTING.md | 82 +++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c9d11f02c87..55c0000a151 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,3 +1,7 @@ + + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 12f6809b3ac..e15eaa82336 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,18 +16,76 @@ 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) + * 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 +projects based on it. This is picked up from both tags on the pull request and +the `Notes: ` annotation in the description. By default, the PR title will be +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: + +*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 +``` + +Projects that you can specify here are: + * matrix-react-sdk + * element-web + * element-desktop + +If your PR introduces a breaking change, you should indicate that in the same +`Notes` section, additionally adding the `pr-breaking` tag (see below). +There's no need to specify in the notes that it's a breaking change - this will +be added automatically based on the tag - 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 tags. + * `pr-breaking`: A breaking change - adding this tag will mean the change causes a *major* version bump. + * `pr-feature`: A new feature - adding this tag 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. + +If you don't have permission to add tags, 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 From ee5a0e7edf8ce3e1c45e227cd4bf9e753fa40571 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 19:47:46 +0100 Subject: [PATCH 02/10] Internal PRs shouldn't have changelog entries by default --- CONTRIBUTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e15eaa82336..35c77f6716b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,7 +45,8 @@ To add a longer, more detailed description of the change for the changelog: 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: +For some PRs, it's not useful to have an entry in the user-facing changelog (this is +the default for PRs tagged with `pr-internal`): *Remove outdated comment from `Ungulates.ts`* ``` @@ -82,7 +83,7 @@ Other metadata can be added using tags. * `pr-breaking`: A breaking change - adding this tag will mean the change causes a *major* version bump. * `pr-feature`: A new feature - adding this tag 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. + * `pr-internal`: No user-facing changes, eg. code comments, CI fixes, refactors or tests. Won't have a changelog entry unless you specify one. If you don't have permission to add tags, your PR reviewer(s) can work with you to add them: ask in the PR description or comments. From b17862e212c61217fac929732d7b66b0917e21e1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 19:55:58 +0100 Subject: [PATCH 03/10] s/tag/label/ Also make breaking change notation clearer. --- CONTRIBUTING.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 35c77f6716b..57bbcdb7ccb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,7 +32,7 @@ Things that should *not* go into your PR description: 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 -projects based on it. This is picked up from both tags on the pull request and +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 used for the changelog entry, but you can specify more options, as follows. @@ -46,7 +46,7 @@ Notes: Fix a bug (https://github.com/matrix-org/notaproject/issues/123) where th ``` For some PRs, it's not useful to have an entry in the user-facing changelog (this is -the default for PRs tagged with `pr-internal`): +the default for PRs labelled with `pr-internal`): *Remove outdated comment from `Ungulates.ts`* ``` @@ -67,11 +67,11 @@ Projects that you can specify here are: * element-web * element-desktop -If your PR introduces a breaking change, you should indicate that in the same -`Notes` section, additionally adding the `pr-breaking` tag (see below). -There's no need to specify in the notes that it's a breaking change - this will -be added automatically based on the tag - but remember to tell the developer how -to migrate: +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 +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* @@ -79,13 +79,13 @@ to migrate: Notes: Remove legacy `Camelopard` class. `Giraffe` should be used instead. ``` -Other metadata can be added using tags. - * `pr-breaking`: A breaking change - adding this tag will mean the change causes a *major* version bump. - * `pr-feature`: A new feature - adding this tag will mean the change causes a *minor* version bump. +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. -If you don't have permission to add tags, your PR reviewer(s) can work with you +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: From fddf0c47fb92bd697787edfb6ba94eae26657dc0 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 21:57:04 +0100 Subject: [PATCH 04/10] Fix formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šimon Brandner --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57bbcdb7ccb..265ea1b90b2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,7 +33,7 @@ Things that should *not* go into your PR description: 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 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 +the `Notes:` annotation in the description. By default, the PR title will be 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: From 9c221419eff86dd43747474bfe6b10099dc46e0b Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 21:57:29 +0100 Subject: [PATCH 05/10] On-brand Co-authored-by: J. Ryan Stinnett --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 265ea1b90b2..48f38548ecf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,7 @@ 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) + * References to any bugs fixed by the change (in GitHub's `Fixes` notation) * Notes for the reviewer that might help them to understand why the change is necessary or how they might better review it. From b0726b5008c8730104266e658e28079f5c0c5462 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 21:57:48 +0100 Subject: [PATCH 06/10] On brand ourself Co-authored-by: J. Ryan Stinnett --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 48f38548ecf..9c54f807479 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,7 +31,7 @@ Things that should *not* go into your PR description: 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 +into the changelogs our users see, both for the JS SDK itself and also for some 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 used for the changelog entry, but you can specify more options, as follows. From c4ccb9d49383f1cc99ab542af1b95068b545f8cb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 22:00:42 +0100 Subject: [PATCH 07/10] Use a real-life example which should hopefully be clearer --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c54f807479..ba0d3af23f9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -59,10 +59,10 @@ 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 +element-web notes: Fix a bug where the 'Herd' button only worked on Tuesdays ``` -Projects that you can specify here are: +This example is for Element Web. You can specify: * matrix-react-sdk * element-web * element-desktop From 5392f616b4f0dc072bece831dc7cd9b4ae518ce8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 22:15:18 +0100 Subject: [PATCH 08/10] Use existing issues labels / scheme Co-authored-by: J. Ryan Stinnett --- CONTRIBUTING.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ba0d3af23f9..8f684e8dcff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,10 +80,10 @@ 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. + * `X-Breaking`: A breaking change - adding this label will mean the change causes a *major* version bump. + * `T-Enhancement`: A new feature - adding this label will mean the change causes a *minor* version bump. + * `T-Defect`: A bug fix (in either code or docs). + * `T-Task`: No user-facing changes, eg. code comments, CI fixes, refactors or tests. Won't have a changelog entry unless you specify one. 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. From 19aedebff1e4e12b59997194c4c8280fffecccef Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 22:16:00 +0100 Subject: [PATCH 09/10] Update other label mentions --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f684e8dcff..0f86aa2fb7f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,7 +46,7 @@ Notes: Fix a bug (https://github.com/matrix-org/notaproject/issues/123) where th ``` 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`): +the default for PRs labelled with `T-Task`): *Remove outdated comment from `Ungulates.ts`* ``` @@ -68,7 +68,7 @@ This example is for Element Web. You can specify: * 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 +way, additionally adding the `X-Breaking` label (see below). There's no need 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: From 45c153321ca21a2a551dc80fcb53188a0df80239 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 14 Jul 2021 22:46:22 +0100 Subject: [PATCH 10/10] Use X-Breaking-Change Apparently we already have this label in react SDK (for some reason) so let's use it. Plus, it's clearer that it's for changes rather than issues that cause things to break. --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f86aa2fb7f..d9050830216 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,7 +68,7 @@ This example is for Element Web. You can specify: * element-desktop If your PR introduces a breaking change, use the `Notes` section in the same -way, additionally adding the `X-Breaking` label (see below). There's no need +way, additionally adding the `X-Breaking-Change` label (see below). There's no need 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: @@ -80,7 +80,7 @@ Notes: Remove legacy `Camelopard` class. `Giraffe` should be used instead. ``` Other metadata can be added using labels. - * `X-Breaking`: A breaking change - adding this label will mean the change causes a *major* version bump. + * `X-Breaking-Change`: A breaking change - adding this label will mean the change causes a *major* version bump. * `T-Enhancement`: A new feature - adding this label will mean the change causes a *minor* version bump. * `T-Defect`: A bug fix (in either code or docs). * `T-Task`: No user-facing changes, eg. code comments, CI fixes, refactors or tests. Won't have a changelog entry unless you specify one.