Skip to content

Add more detail on the context/rationale that should be included when contributing #2432

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 7, 2022
Merged

Add more detail on the context/rationale that should be included when contributing #2432

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
792321b
More detail on the context/rationale that should be included when con…
MadLittleMods Jun 2, 2022
a31a02a
Prefer issue and comments in code
MadLittleMods Jun 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,25 @@ 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)
* Describe the why and what is changing in the PR description so it's easy for
onlookers and reviewers to onboard and context switch.
onlookers and reviewers to onboard and context switch. This information is
also helpful when we come to look at this in 6 months and ask "why did we do
it like that?" we have a chance of finding out.
* Why is the change necessary? What problem does it solve? Why didn't it
work before? Why does it work now? What use cases does it unlock?
* If you find yourself adding information on how the code works or why you
chose to do it the way you did, make sure this information is also
written as comments in the code itself.
* Sometimes a PR can change considerably as it is developed. In this case,
the description should be updated to reflect the most recent state of
the PR. (It can be helpful to retain the old content under a suitable
heading, for additional context.)
* Include both **before** and **after** screenshots to easily compare and discuss
what's changing.
* Include a step-by-step testing strategy so that a reviewer can check out the
code locally and easily get to the point of testing your change.
* Add comments to the diff for the reviewer that might help them to understand
why the change is necessary or how they might better understand and 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 labels on the pull request and
Expand Down Expand Up @@ -244,6 +250,12 @@ on Git 2.17+ you can mass signoff using rebase:
git rebase --signoff origin/develop
```

Review expectations
===================

See https://github.com/vector-im/element-meta/wiki/Review-process


Merge Strategy
==============

Expand Down