Skip to content

Commit

Permalink
Merge pull request bitcoin#14 from taoeffect/patch-1
Browse files Browse the repository at this point in the history
Updated "Decision Making" section to reflect current uncertainty
  • Loading branch information
ahmedbodi authored and ahmedbodi committed Jan 18, 2016
2 parents b1cf005 + 8e1fe6c commit e2cb32f
Showing 1 changed file with 4 additions and 34 deletions.
38 changes: 4 additions & 34 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,42 +67,12 @@ Project maintainers aim for a quick turnaround on refactoring pull requests, so
"Decision Making" Process
-------------------------

The following applies to code changes to the Bitcoin Core project (and related projects such as libsecp256k1), and is not to be confused with overall Bitcoin Network Protocol consensus changes.
TBD.

Whether a pull request is merged into Bitcoin Core rests with the project merge maintainers and ultimately the project lead.

Maintainers will take into consideration if a patch is in line with the general principles of the project; meets the minimum standards for inclusion; and will judge the general consensus of contributors.

In general, all pull requests must:

- have a clear use case, fix a demonstrable bug or serve the greater good of the project (for example refactoring for modularisation);
- be well peer reviewed;
- have unit tests and functional tests where appropriate;
- follow code style guidelines;
- not break the existing test suite;
- where bugs are fixed, where possible, there should be unit tests demonstrating the bug and also proving the fix. This helps prevent regression.

Patches that change Bitcoin consensus rules are considerably more involved than normal because they affect the entire ecosystem and so must be preceded by extensive mailing list discussions and have a numbered BIP. While each case will be different, one should be prepared to expend more time and effort than for other kinds of patches because of increased peer review and consensus building requirements.


###Peer Review

Anyone may participate in peer review which is expressed by comments in the pull request. Typically reviewers will review the code for obvious errors, as well as test out the patch set and opine on the technical merits of the patch. Project 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). The following language is used within pull-request comments:

- ACK means "I have tested the code and I agree it should be merged";
- NACK means "I disagree this should be merged", and must be accompanied by sound technical justification. NACKs without accompanying reasoning may be disregarded;
- utACK means "I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged";
- Concept ACK means "I agree in the general principle of this pull request";
- Nit refers to trivial, often non-blocking issues.

Reviewers should include the commit hash which they reviewed in their comments.

Project maintainers reserve the right to weigh the opinions of peer reviewers using common sense judgement and also may weight based on meritocracy: Those that have demonstrated a deeper commitment and understanding towards the project (over time) or have clear domain expertise may naturally have more weight, as one would expect in all walks of life.

Where a patch set affects consensus critical code, the bar will be set much higher in terms of discussion and peer review requirements, keeping in mind that mistakes could be very costly to the wider community. This includes refactoring of consensus critical code.

Where a patch set proposes to change the Bitcoin consensus, it must have been discussed extensively on the mailing list and IRC, be accompanied by a widely discussed BIP and have a generally widely perceived technical consensus of being a worthwhile change based on the judgement of the maintainers.
Relevant links:

- https://github.com/bitcoinclassic/bitcoinclassic/issues/13
- https://bitcoinclassic.consider.it/how-to-change-the-code?results=true

Release Policy
--------------
Expand Down

0 comments on commit e2cb32f

Please sign in to comment.