diff options
Diffstat (limited to 'CONTRIBUTING.md')
| -rw-r--r-- | CONTRIBUTING.md | 240 |
1 files changed, 240 insertions, 0 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..fc8d58d97 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,240 @@ +Contributing to Bitcoin Core +============================ + +The Bitcoin Core project operates an open contributor model where anyone is +welcome to contribute towards development in the form of peer review, testing +and patches. This document explains the practical process and guidelines for +contributing. + +Firstly in terms of structure, there is no particular concept of "Core +developers" in the sense of privileged people. Open source often naturally +revolves around meritocracy where longer term contributors gain more trust from +the developer community. However, 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, overall merging, moderation and appointment of maintainers. + + +Contributor Workflow +-------------------- + +The codebase is maintained using the "contributor workflow" where everyone +without exception contributes patch proposals using "pull requests". This +facilitates social contribution, easy testing and peer review. + +To contribute a patch, the workflow is as follows: + + - Fork repository + - Create topic branch + - Commit patches + +The project coding conventions in the [developer notes](doc/developer-notes.md) +must be adhered to. + +In general [commits should be atomic](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) +and diffs should be easy to read. For this reason do not mix any formatting +fixes or code moves with actual code changes. + +Commit messages should be verbose by default consisting of a short subject line +(50 chars max), a blank line and detailed explanatory text as separate +paragraph(s); unless the title alone is self-explanatory (like "Corrected typo +in init.cpp") then a single title line is sufficient. Commit messages should be +helpful to people reading your code in the future, so explain the reasoning for +your decisions. Further explanation [here](http://chris.beams.io/posts/git-commit/). + +If a particular commit references another issue, please add the reference, for +example `refs #1234`, or `fixes #4321`. Using the `fixes` or `closes` keywords +will cause the corresponding issue to be closed when the pull request is merged. + +Please refer to the [Git manual](https://git-scm.com/doc) for more information +about Git. + + - Push changes to your fork + - Create pull request + +The title of the pull request should be prefixed by the component or area that +the pull request affects. Valid areas as: + + - *Consensus* for changes to consensus critical code + - *Docs* for changes to the documentation + - *Qt* for changes to bitcoin-qt + - *Mining* for changes to the mining code + - *Net* or *P2P* for changes to the peer-to-peer network code + - *RPC/REST/ZMQ* for changes to the RPC, REST or ZMQ APIs + - *Scripts and tools* for changes to the scripts and tools + - *Tests* for changes to the bitcoin unit tests or QA tests + - *Trivial* should **only** be used for PRs that do not change generated + executable code. Notably, refactors (change of function arguments and code + reorganization) and changes in behavior should **not** be marked as trivial. + Examples of trivial PRs are changes to: + - comments + - whitespace + - variable names + - logging and messages + - *Utils and libraries* for changes to the utils and libraries + - *Wallet* for changes to the wallet code + +Examples: + + Consensus: Add new opcode for BIP-XXXX OP_CHECKAWESOMESIG + Net: Automatically create hidden service, listen on Tor + Qt: Add feed bump button + Trivial: Fix typo in init.cpp + +If a pull request is specifically not to be considered for merging (yet) please +prefix the title with [WIP] or use [Tasks Lists](https://help.github.com/articles/basic-writing-and-formatting-syntax/#task-lists) +in the body of the pull request to indicate tasks are pending. + +The body of the pull request should contain enough description about what the +patch does together with any justification/reasoning. You should include +references to any discussions (for example other tickets or mailing list +discussions). + +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. + +Squashing Commits +--------------------------- +If your pull request is accepted for merging, you may be asked by a maintainer +to squash and or [rebase](https://git-scm.com/docs/git-rebase) your commits +before it will be merged. The basic squashing workflow is shown below. + + git checkout your_branch_name + git rebase -i HEAD~n + # n is normally the number of commits in the pull + # set commits from 'pick' to 'squash', save and quit + # on the next screen, edit/refine commit messages + # save and quit + git push -f # (force push to GitHub) + +If you have problems with squashing (or other workflows with `git`), you can +alternatively enable "Allow edits from maintainers" in the right GitHub +sidebar and ask for help in the pull request. + +Please refrain from creating several pull requests for the same change. +Use the pull request that is already open (or was created earlier) to amend +changes. This preserves the discussion and review that happened earlier for +the respective change set. + +The length of time required for peer review is unpredictable and will vary from +pull request to pull request. + + +Pull Request Philosophy +----------------------- + +Patchsets should always be focused. For example, a pull request could add a +feature, fix a bug, or refactor code; but not a mixture. Please also avoid super +pull requests which attempt to do too much, are overly large, or overly complex +as this makes review difficult. + + +###Features + +When adding a new feature, thought must be given to the long term technical debt +and maintenance that feature may require after inclusion. Before proposing a new +feature that will require maintenance, please consider if you are willing to +maintain it (including bug fixing). If features get orphaned with no maintainer +in the future, they may be removed by the Repository Maintainer. + + +###Refactoring + +Refactoring is a necessary part of any software project's evolution. The +following guidelines cover refactoring pull requests for the project. + +There are three categories of refactoring, code only moves, code style fixes, +code refactoring. In general refactoring pull requests should not mix these +three kinds of activity in order to make refactoring pull requests easy to +review and uncontroversial. In all cases, refactoring PRs must not change the +behaviour of code within the pull request (bugs must be preserved as is). + +Project maintainers aim for a quick turnaround on refactoring pull requests, so +where possible keep them short, uncomplex and easy to verify. + + +"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. + +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 (or in certain cases of copyright/patent/licensing + issues, legal 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. + + +Release Policy +-------------- + +The project leader is the release manager for each Bitcoin Core release. + +Copyright +--------- + +By contributing to this repository, you agree to license your work under the +MIT license unless specified otherwise in `contrib/debian/copyright` or at +the top of the file itself. Any work contributed where you are not the original +author must contain its license header with the original author(s) and source. |