Merge pull request #2111 from rnicoll/contributing
Refresh `CONTRIBUTING.MD` to fit Dogecoin
This commit is contained in:
Ross Nicoll
GitHub
commit
ce54d88b56
195
CONTRIBUTING.md
195
CONTRIBUTING.md
|
|
@ -1,18 +1,20 @@
|
|||
# Contributing to Dogecoin Core
|
||||
|
||||
The Dogecoin 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.
|
||||
Dogecoin Core is open source software, and we would welcome contributions
|
||||
which improve the state of the software. For those wanting to discuss changes,
|
||||
or look for work that needs doing, please see:
|
||||
|
||||
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.
|
||||
* [Help requests](https://github.com/dogecoin/dogecoin/labels/help%20wanted)
|
||||
* [Projects](https://github.com/dogecoin/dogecoin/projects)
|
||||
* [Dogecoindev on reddit](https://www.reddit.com/r/dogecoindev/)
|
||||
|
||||
## Branch Strategy
|
||||
|
||||
Dogecoin Core's default branch is intentionally a stable release, so that anyone
|
||||
downloading the code and compiling it gets a stable release. Active development
|
||||
occurs on branches named after the version they are targeting, for example the
|
||||
1.14.4 branch is named `1.14.4-dev`. When raising PRs, please raise against the
|
||||
relevant development branch and **not** against the `master` branch.
|
||||
|
||||
## Contributor Workflow
|
||||
|
||||
|
|
@ -22,12 +24,15 @@ facilitates social contribution, easy testing and peer review.
|
|||
|
||||
To contribute a patch, the workflow is as follows:
|
||||
|
||||
- Fork repository
|
||||
- Create topic branch
|
||||
- Commit patches
|
||||
- Fork the repository in GitHub, and clone it your development machine.
|
||||
- Create a topic branch from the relevant development branch.
|
||||
- Commit changes to the branch.
|
||||
- Test your changes, which **must** include the unit and RPC tests passing.
|
||||
- Push topic branch to your copy of the repository.
|
||||
- Raise a Pull Request via GitHub.
|
||||
|
||||
The project coding conventions in the [developer notes](doc/developer-notes.md)
|
||||
must be adhered to.
|
||||
The 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
|
||||
|
|
@ -40,57 +45,15 @@ 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 dogecoin-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 dogecoin 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.
|
||||
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 feedback.
|
||||
|
||||
|
||||
## Squashing Commits
|
||||
|
|
@ -116,15 +79,15 @@ 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.
|
||||
The length of time required for peer review is unpredictable and will vary
|
||||
between pull requests.
|
||||
|
||||
|
||||
## 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
|
||||
Pull Requests should always be focused. For example, a pull request could add a
|
||||
feature, fix a bug, or refactor code; but not a mixture. Please avoid submitting
|
||||
pull requests that attempt to do too much, are overly large, or overly complex
|
||||
as this makes review difficult.
|
||||
|
||||
|
||||
|
|
@ -134,99 +97,77 @@ 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.
|
||||
in the future, they may be removed.
|
||||
|
||||
|
||||
### Refactoring
|
||||
|
||||
Refactoring is a necessary part of any software project's evolution. The
|
||||
following guidelines cover refactoring pull requests for the project.
|
||||
Dogecoin Core is a direct fork of Bitcoin Core and therefore benefits from as
|
||||
little refactoring as possible on code that is created upstream. If you see any
|
||||
structural issues with upstream code, please propose these fixes for
|
||||
[bitcoin/bitcoin](https://github.com/bitcoin/bitcoin) and future Dogecoin Core
|
||||
releases will automatically benefit from these.
|
||||
|
||||
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.
|
||||
When refactoring Dogecoin-specific code, please keep refactoring requests short,
|
||||
low complexity and easy to verify.
|
||||
|
||||
|
||||
## "Decision Making" Process
|
||||
|
||||
The following applies to code changes to the Dogecoin Core project (and related
|
||||
projects such as libsecp256k1), and is not to be confused with overall Dogecoin
|
||||
Network Protocol consensus changes.
|
||||
The following applies to code changes to Dogecoin Core, and is not to be
|
||||
confused with overall Dogecoin Network Protocol consensus changes. All consensus
|
||||
changes **must** be ratified by miners; a proposal to implement protocol changes
|
||||
does not guarantee activation on the mainnet, not even when a binary gets
|
||||
released by maintainers.
|
||||
|
||||
Whether a pull request is merged into Dogecoin Core rests with the project merge
|
||||
maintainers and ultimately the project lead.
|
||||
Whether a pull request is merged into Dogecoin Core rests with the repository
|
||||
maintainers.
|
||||
|
||||
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.
|
||||
principles of Dogecoin; meets the minimum standards for inclusion; and will
|
||||
take into account the consensus among frequent 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;
|
||||
Dogecoin;
|
||||
- be peer reviewed;
|
||||
- have unit tests and functional tests;
|
||||
- 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.
|
||||
demonstrating the bug and also proving the fix. This helps prevent
|
||||
regressions.
|
||||
|
||||
Patches that change Dogecoin 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.
|
||||
The following patch types are expected to have significant discussion before
|
||||
approval and merge:
|
||||
|
||||
- Consensus rule changes (through softfork or otherwise)
|
||||
- Policy changes
|
||||
|
||||
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:
|
||||
test out the patch set and opine on the technical merits of the patch.
|
||||
Repository maintainers take into account the peer review when determining if
|
||||
there is consensus to merge a pull request.
|
||||
|
||||
- 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
|
||||
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
|
||||
that have demonstrated a deeper commitment and understanding towards Dogecoin
|
||||
(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 Dogecoin 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 Dogecoin Core release.
|
||||
|
||||
discussed extensively, be accompanied by widely discussed documentation and have
|
||||
a generally widely perceived technical consensus of being a worthwhile change,
|
||||
based on the judgement of the maintainers.
|
||||
|
||||
## Copyright
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue