This repository was archived by the owner on Mar 5, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add PR and release guidelines #3224
Merged
Merged
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
90c4cc5
Add contributions guidelines
cgewecke 067e526
Reword a couple things
cgewecke 278112e
Add note about PR descriptions
cgewecke fb503e8
Set review time window
cgewecke 480e54d
3 day review
cgewecke 2c59606
Titles and number reformatting
cgewecke f7883fb
Refer to emergencies discussion at js-organization
cgewecke fe62312
Reformat
cgewecke 2b2323b
Document # of approvals
cgewecke 0aad9b7
Merge branch '1.x' into docs/prs-and-releases
nivida 89704e1
Lengthen release review to 1 week, require stylistic consistency
cgewecke 13e78b5
Merge branch '1.x' into docs/prs-and-releases
nivida c1e2a16
Merge branch '1.x' into docs/prs-and-releases
nivida File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
### Guidelines for Pull Requests and Releases (Web3 1.x) | ||
|
||
This document provides some ground rules for contributors (including the maintainer(s) of | ||
the project) about how to make, review and publish changes to 1.x. The most basic requirement is | ||
that **Web3 not break**. | ||
|
||
## Pull Requests for substantive changes (e.g. everything except comments and docs) | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
1. Any PR that introduces a logic change should include tests. (In many cases, the tests will take | ||
more time to write than the actual code). | ||
|
||
2. All PRs should sit for 48 hours with the `pleasereview` tag in order to garner feedback. | ||
nivida marked this conversation as resolved.
Show resolved
Hide resolved
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
3. No PR should be merged until it has been reviewed, passes CI, and all reviews' comments are | ||
addressed. | ||
|
||
4. PRs should have a narrow, well-defined focus and make the smallest set of changes possible to achieve their goal. They should include a clear description in the opening comment. | ||
nivida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
5. Given the choice between a conservative change that mostly works and an adventurous change which | ||
seems better but introduces uncertainty - prefer the conservative change. | ||
|
||
nivida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Reviews | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The end-goal of review is to suggest useful improvements to the author. Reviews should finish with | ||
approval unless there are issues that would result in: | ||
|
||
1. Buggy behaviour. | ||
|
||
2. Undue maintenance burden. | ||
|
||
3. Pessimisation (i.e. speed reduction / meaningful build-size increases). | ||
|
||
4. Feature reduction (i.e. it removes some aspect of functionality that users rely on). | ||
|
||
5. Avoidable risk (i.e it's difficult to test or hard to anticipate the implications of, without | ||
being strictly necessary to fix something broken). | ||
|
||
## Releases | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
All releases should be proposed in a PR and subject to community review for a minimum of 72 hours (3 days). | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
2. Release review periods should be accompanied by a published `rc` version which can be used for | ||
sanity checks / additional testing. | ||
|
||
3. During release review, the code is frozen unless new changes are proposed, approved and merged. | ||
Changes reset the release clock and should trigger a new `rc` release. | ||
|
||
4. Regular maintainers should manually test the `rc` against a Node project and the published | ||
minified bundle in a browser context. An external reviewer should verify they've done the same. | ||
|
||
nivida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Emergencies | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[THIS SECTION NEEDS WORK after discussing the release procedure more...] | ||
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
cgewecke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
(Much of the above is borrowed from Openish, Parity and Ethers contributions docs. It's meant | ||
to establish clear, egalitarian criteria for making changes to the code while prioritizing the | ||
safety of Web3's users.) | ||
|
||
|
||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.