diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f92516750..0e0e93f16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,18 +1,20 @@ # Contributing to Dogecoin Core -Dogecoin Core is an open source project, and we would welcome contributions which provably -improve the state of the software. For those wanting to discuss changes, or look for work -that needs doing, please see: +Dogecoin Core is open source software, and we would welcome contributions +which improves the state of the software. For those wanting to discuss changes, +or look for work that needs doing, please see: +* [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/) -* [Discord](https://discord.gg/dogecoin) (in particular the #core-dev channel) ## 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 targetting, for example the 1.14.4 branch is named `1.14.4-dev`. -When raising PRs, please raise against the relevant development branch, **not** against the `master` branch. +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,18 +24,15 @@ facilitates social contribution, easy testing and peer review. To contribute a patch, the workflow is as follows: - - Fork repository in GitHub, and clone it your development machine. - - Create topic branch from the relevant development branch. - - As these branches are in the contributor's local repository, naming is not critical, - although it is recommended that you include the target version. If the change relates - to an issue, including its number in the branch name is also a good idea. - - Commit patches to the branch. - - Test your changes, which **must** include the unit and RPC tests passing. Changes will not be accepted if they do not pass tests. + - 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 PR via GitHub. + - 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 @@ -46,24 +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 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 @@ -89,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. @@ -107,98 +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 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 +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