Skip to content

Commit 792321b

Browse files
committed
More detail on the context/rationale that should be included when contributing
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 b64dbdc commit 792321b

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 to look at this in 6 months and ask "why did we do
28+
it like that?" we have a chance of finding out.
29+
* Why is the change necessary? What problem does it solve? Why didn't it
30+
work before? Why does it work now? What use cases does it 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 also
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
@@ -244,6 +250,12 @@ on Git 2.17+ you can mass signoff using rebase:
244250
git rebase --signoff origin/develop
245251
```
246252

253+
Review expectations
254+
===================
255+
256+
See https://github.com/vector-im/element-meta/wiki/Review-process
257+
258+
247259
Merge Strategy
248260
==============
249261

0 commit comments

Comments
 (0)