From 98f7e82d07182d3caaacd1f3330f3b8cd84559a4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 25 Feb 2022 11:49:49 +0000 Subject: [PATCH] Merge bitcoin/bitcoin#24433: doc: Explain that feedback needs to be addressed fa694f61ab21cb973843e5234a6aeacd4a957e05 doc: Explain that feedback needs to be addressed (MarcoFalke) fa0819eea380047c9d7404895d3e4e8ea37930bb doc: Move peer-review paragraph to right section (MarcoFalke) fa2b65b3583e7e7ffa892bf2453720b0d2bd7904 doc: Add link to release-process.md in CONTRIBUTING.md (MarcoFalke) Pull request description: Generally, the pull request author is expected to reply to all comments or iterate the code before merge. Of course, it is allowed to reject feedback, but it should not be done by silently ignoring it. Clarify this in the docs. Also, some minor copy edits. ACKs for top commit: michaelfolkson: ACK fa694f61ab21cb973843e5234a6aeacd4a957e05 Sjors: ACK fa694f61ab21cb973843e5234a6aeacd4a957e05 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/24433/commits/fa694f61ab21cb973843e5234a6aeacd4a957e05 prayank23: ACK https://github.com/bitcoin/bitcoin/pull/24433/commits/fa694f61ab21cb973843e5234a6aeacd4a957e05 brunoerg: ACK fa694f61ab21cb973843e5234a6aeacd4a957e05 w0xlt: ACK fa694f6 Tree-SHA512: 339d6f252395664442f4bfb73d839314de8c9b0fdc8900a1c4a67b1cef9c73ecb98c7587a842dd5a36a4a3efbd8270d2162962013893706313f7ef34491db18c --- CONTRIBUTING.md | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a075ee3a6c..d181a70746 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,8 +12,8 @@ revolves around a meritocracy where contributors earn trust from the developer community over time. Nevertheless, some hierarchy is necessary for practical purposes. As such, there are repository "maintainers" who are responsible for merging pull requests, as well as a "lead maintainer" who is responsible for the -release cycle as well as overall merging, moderation and appointment of -maintainers. +[release cycle](/doc/release-process.md) as well as overall merging, moderation +and appointment of maintainers. Getting Started --------------- @@ -168,9 +168,14 @@ in the body of the pull request to indicate tasks are pending. At this stage, one should expect comments and review from other contributors. You can add more commits to your pull request by committing them locally and pushing -to your fork until you have satisfied all feedback. +to your fork. -Note: Code review is a burdensome but important part of the development process, and as such, certain types of pull requests are rejected. In general, if the **improvements** do not warrant the **review effort** required, the PR has a high chance of being rejected. It is up to the PR author to convince the reviewers that the changes warrant the review effort, and if reviewers are "Concept NACK'ing" the PR, the author may need to present arguments and/or do research backing their suggested changes. +You are expected to reply to any review comments before your pull request is +merged. You may update the code or reject the feedback if you do not agree with +it, but you should express so in a reply. If there is outstanding feedback and +you are not actively working on it, your pull request may be closed. + +Please refer to the [peer review](#peer-review) section below for more details. ### Squashing Commits @@ -301,6 +306,14 @@ maintainers take into account the peer review when determining if there is consensus to merge a pull request (remember that discussions may have been spread out over GitHub, mailing list and IRC discussions). +Code review is a burdensome but important part of the development process, and +as such, certain types of pull requests are rejected. In general, if the +**improvements** do not warrant the **review effort** required, the PR has a +high chance of being rejected. It is up to the PR author to convince the +reviewers that the changes warrant the review effort, and if reviewers are +"Concept NACK'ing" the PR, the author may need to present arguments and/or do +research backing their suggested changes. + #### Conceptual Review A review can be a conceptual review, where the reviewer leaves a comment @@ -481,11 +494,6 @@ the contents onto the [Tracker](https://docs.google.com/spreadsheets/d/1DnKxat0S When pasting the contents, make sure to split the values into the cells so every line is not present under commit hash. -Release Policy --------------- - -The project leader is the release manager for each Dash Core release. - Copyright ---------