From 48abb72eacc8ddff40128fd8d55add999df9290d Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:13:15 +0200 Subject: [PATCH 1/9] [docs] change channels for interactions Removes discord because this is a chat channel and not suitable to make decisions that are auditable. Adds a link to "help wanted" in the github repository as those are the most pressing items that help Dogecoin --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f92516750..a94b058cf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,9 +4,9 @@ Dogecoin Core is an open source project, and we would welcome contributions whic improve 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 From cfb9ecdc58a0fc5a0d121c46840a1d63061b5978 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:17:52 +0200 Subject: [PATCH 2/9] [docs] Make the workflow more concise and less chatty --- CONTRIBUTING.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a94b058cf..bdb6baa6c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,15 +22,12 @@ 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. From 97edf2a6b06b03615a518eb3ac9969a813717fb2 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:24:58 +0200 Subject: [PATCH 3/9] [docs] Clarify PR requirements and assessment process. --- CONTRIBUTING.md | 84 +++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 51 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bdb6baa6c..5e28bbd36 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -92,9 +92,9 @@ 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 +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. @@ -124,78 +124,60 @@ where possible keep them short, uncomplex 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 From 158aff28971aef46e4527b74ac06cc34f7f8f1a4 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:28:48 +0200 Subject: [PATCH 4/9] [docs] reformat branching strategy in contribution guide --- CONTRIBUTING.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5e28bbd36..2943e062c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,9 +10,11 @@ that needs doing, please see: ## 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 From c701a2fc6ff211ae54f8dc05b96c96b8f0a6e49c Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:30:31 +0200 Subject: [PATCH 5/9] [docs] Clean up introduction --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2943e062c..f35f1f054 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,8 +1,8 @@ # 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) From 4175bf45adf2cbf6c893c77292367d6e8786fea0 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:31:38 +0200 Subject: [PATCH 6/9] [docs] Clean up Contributor Workflow --- CONTRIBUTING.md | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f35f1f054..261634009 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,8 +31,8 @@ To contribute a patch, the workflow is as follows: - 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 @@ -45,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 From fe5dab6a60b33fdf93127fa22c19d716a8ee5b0e Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:32:44 +0200 Subject: [PATCH 7/9] [docs] Clean up squashing section --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 261634009..124deab7a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,8 +79,8 @@ 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 From 3c6a25fb7c67a6f0d3099940cae294bebc18d9a2 Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:34:50 +0200 Subject: [PATCH 8/9] [docs] Change Refactoring policy --- CONTRIBUTING.md | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 124deab7a..17e3b8193 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,17 +102,14 @@ 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. +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 From a485280a7596818e467bbb3efeb7d7a7384c541c Mon Sep 17 00:00:00 2001 From: Patrick Lodder Date: Thu, 3 Jun 2021 19:36:12 +0200 Subject: [PATCH 9/9] [docs] Allow anyone to remove something dysfunctional --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 17e3b8193..0e0e93f16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,7 +97,7 @@ 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