Skip to content

Commit 96c35e2

Browse files
Add more detail on the context/rationale that should be included when contributing (#2432)
Follow-up to #1933 Spawning from various recent documents and comments: - https://github.com/vector-im/element-meta/wiki/Review-process - matrix-org/synapse#12846 (comment) - https://gitlab.matrix.org/new-vector/internal/-/wikis/Backend/Reviews
1 parent cb5b2e1 commit 96c35e2

File tree

1 file changed

+18
-6
lines changed

1 file changed

+18
-6
lines changed

CONTRIBUTING.md

+18-6
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,25 @@ Things that should go into your PR description:
2323
* A changelog entry in the `Notes` section (see below)
2424
* References to any bugs fixed by the change (in GitHub's `Fixes` notation)
2525
* Describe the why and what is changing in the PR description so it's easy for
26-
onlookers and reviewers to onboard and context switch.
26+
onlookers and reviewers to onboard and context switch. This information is
27+
also helpful when we come back to look at this in 6 months and ask "why did
28+
we do it like that?" we have a chance of finding out.
29+
* Why didn't it work before? Why does it work now? What use cases does it
30+
unlock?
31+
* If you find yourself adding information on how the code works or why you
32+
chose to do it the way you did, make sure this information is instead
33+
written as comments in the code itself.
34+
* Sometimes a PR can change considerably as it is developed. In this case,
35+
the description should be updated to reflect the most recent state of
36+
the PR. (It can be helpful to retain the old content under a suitable
37+
heading, for additional context.)
2738
* Include both **before** and **after** screenshots to easily compare and discuss
2839
what's changing.
2940
* Include a step-by-step testing strategy so that a reviewer can check out the
3041
code locally and easily get to the point of testing your change.
3142
* Add comments to the diff for the reviewer that might help them to understand
3243
why the change is necessary or how they might better understand and review it.
3344

34-
Things that should *not* go into your PR description:
35-
* Any information on how the code works or why you chose to do it the way
36-
you did. If this isn't obvious from your code, you haven't written enough
37-
comments.
38-
3945
We rely on information in pull request to populate the information that goes
4046
into the changelogs our users see, both for the JS SDK itself and also for some
4147
projects based on it. This is picked up from both labels on the pull request and
@@ -254,6 +260,12 @@ on Git 2.17+ you can mass signoff using rebase:
254260
git rebase --signoff origin/develop
255261
```
256262

263+
Review expectations
264+
===================
265+
266+
See https://github.com/vector-im/element-meta/wiki/Review-process
267+
268+
257269
Merge Strategy
258270
==============
259271

0 commit comments

Comments
 (0)