Merge pull request #20 from patricklodder/concise-contribution-guide

Make contribution guide more concise
This commit is contained in:
Ross Nicoll 2021-06-03 20:03:09 +01:00 committed by GitHub
commit 2e81416cf5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -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