BEFORE YOU BEGIN
This guide builds on the foundation laid by How to Review Pull Requests in Bitcoin Core.
Remember that testing issues and reviewing are the most effective ways you can contribute as a new contributor (and will teach you more about the codebase than opening PRs).
Don't be in a hurry to add to the growing pile of open PRs.
A good rule of thumb is to review 5-15 PRs (or solve or test 5-15 open issues) for every PR you make.
HOW TO CONTRIBUTE PULL REQUESTS TO BITCOIN CORE
When you do start contributing PRs, here are some guidelines:
Aim for quality over quantity and a balance between deep work and quick wins.
What to do
Choose your contribution carefully to be sure it is desired by the maintainers and reviewers; without their approval, you risk squandering your time and social capital.
The best places to look for good things to do as a new contributor are:
Things to do and fix can also be found by searching for TODO and
FIXME comments in the codebase. An easy way to do this is to
git grep "TODO\|FIXME" on the command line from
root. Keep in mind that these comments can sometimes be out of
Focus on user problems, actual bugs, and "used, but untested" methods that affect outcomes and need tests. Bitcoin Core is a large, legacy codebase and there are inconsistencies, but it's good to keep a healthy perspective on priorities. An item might seem fun to contribute, but before opening that PR, consider the review time you're asking for. Because of your PR, is a regression or CVE getting past a lack of review elsewhere?
Avoid making trivial refactoring, style cleanup, or spelling PRs; these consume valuable contributor and maintainer time, often for little gain. Code churn can cause hidden new bugs and blurs the code history and git blame. Any code refactoring needs substantial motivation, advantages, or simplifications.
Do not begin by trying to change consensus code; it is difficult and dangerous territory. The goal of Bitcoin Core is to maintain the correct consensus on the Bitcoin network — all else is secondary.
As a general guideline, here are priorities in decreasing order: fixing bugs, improving security/test coverage, addressing real use cases, doing less/removing code (only when doing nothing isn't better), doing more/adding code.
Gather context about the change you have in mind. Has it already
been proposed in the past? Do issues or PRs about it already
exist? (They probably do). Sleuth a bit. Search the
log -S history. Look at the
git blame of the
code in question and
git show the commits. Check
the GitHub file history. You'll see who understands that part of
the codebase and can be contacted for questions or review.
It may save you a lot of time to consult with relevant people on IRC before tackling a difficult project in potentially the wrong way, or one that has already been tried and turned out to be a dead end for reasons that may not be apparent to you.
I sometimes seek Concept ACKs for larger changes at IRC meetings (example) as a topic request and discussion.
Write test coverage
A Gregory Maxwell warning to test contributors: "Overly exact tests have greatly diminished value if their exactitude turns them into hyper-active something changed detectors, when they would otherwise be more useful as something isn't right detectors if constructed more in terms of invariants instead of strict behaviour."
Documentation is important, e.g. high-level documentation of how
things work and interact, clear and accurate code docs, whether
a function has a good description and
test logging (both
and so on.
when appropriate. RPC-related release notes should refer to the
RPC help for details instead of substituting for full
documentation, for example: "Please refer to the RPC help
getbalances for details."
To avoid potential merge conflicts, release notes are usually
added in a separate markdown file in the
not directly in
In general, PRs that intelligently improve documentation and tests in a well thought-out way tend to be well-received.
Make sure each individual commit in your PR is hygienic. Every commit must compile successfully on its own without warnings, errors, regressions, or test failures.
This means compiling your PRs with
passed to your build configuration. Run
./configure --help | grep -A1 --enable-debug
for more info.
Set up Travis CI on your own GitHub Bitcoin repository so that when you push a branch or commit, the linter and Travis CI (continuous integration) tests run. It can be a good idea to verify that they are all green on your GitHub repository before filing a PR. Unfortunately, Travis CI is also sometimes slow and times out frequently — you may have to look at the logs of failing jobs to distinguish between real issues and when you just need to restart the job.
When it works, https://bitcoinbuilds.org runs more quickly than Travis CI (but for the moment it tests fewer platforms), so don't hesitate to consult it for quicker feedback when you push a PR or changes.
Do it well
Don't rush. Give your work time to mature, revisit it with fresh perspective, and iterate on it a few times before putting it out there and asking for others' time. It might be a good rule of thumb to write several WIP PRs for each one that you do file.
Put the time in to make your PRs as easy to understand and review as possible: Code, tests, documentation, good commit messages, relevant references, concise explanations.
Consider also the timing of what is going on in the project in terms of priorities and releases as well as your current status. If you already have a few PRs open it may be best to close them or concentrate on having them merged before adding yet another PR to the review stack. The idea of limiting open PRs to 5 per contributor is occasionally raised; consider staying well under that.
Give some thought to PR size and scope. Many contributors err on the side of making their PRs too large and time-consuming to review. It is often better to keep your PRs short and focused to make them easier and safer to review. This may require splitting your work into several consecutive PRs.
As described in the Bitcoin Core
prefix your PR title with the relevant area
PR descriptions are important. They "sell" your PR yet are often either too terse or too verbose. Write the essentials, then take the time to make your PR description brief, clear, even pithy.
Never put @-prefixed usernames in commits and PR descriptions; the latter because usernames in the description are copied into the merge commit by the merge script. This can cause endless annoying notifications for those concerned. An update to the merge script now warns maintainers if a merge message contains a @username, but it's better not to add them to begin with.
Don't rely on markdown in your PR description for it to make sense, especially strike-through formatting. The Bitcoin Core merge script will copy your PR description to git history in the merge commit but any formatting will be lost.
PR descriptions need to explain why. Summarizing "what" can be good too, but "why" is essential to review because if the why doesn't make sense then the what probably doesn't matter. Sometimes things seem self-apparent, but when in doubt no one was ever hurt by a little more concise explanation.
Say how to review and test your PR
It's a smart habit to give reviewers tips in your PR description on how to review and test your changes: "Here's how to review this".
It essential to be comfortable with frequently rebasing your commits to reorganise or squash them. Rebasing is frequently done to keep your commits logical, separate, and easier to review. Even better, hygienic — each standing on their own without introducing regressions.
For simple edits of existing commits, don't hesitate to squash your commits before pushing, without waiting for the maintainers to ask you to do so.
You can sign your commits using the OpenTimestamps Git Integration. It's easy to setup.
Remember that every time you comment on your PR after pushing, you're sending notifications to the people who previously reviewed or commented on it. Respect their time.
Learn how to get people to review your work when needed. It's a skill. Review is scarce and valuable -- it's the bottleneck.
The best ways to get review are organic: Making sure your work is interesting, desired, and done well. Giving back by helping others and giving review. Giving more than you take or ask for; adding value as best as you can while figuring things out on your own. It's a long and sometimes frustrating path but a rewarding one.
While waiting for review, the very best thing you can do is give review to others. Karma is a thing — and reviewing/testing PRs and issues builds your credibility and understanding of the codebase and the process.
Persistence can pay off. Sometimes a PR succeeds on the second or third try if the first ones didn't see enough review and if there are valid reasons to continue.
In this case, one idea seen in practice is to recap various ACKs from the previous PRs, with GitHub usernames, to rope in support for the new PR. If you do, be sure to do it in a comment — not in a commit message and not in the PR description.
Ask yourself how others will see your PR. Try to only file PRs that people will be motivated to review and that are worth merging, for both Bitcoin and your track record. Your PRs will receive more attention from the contributors and maintainers and be merged more quickly.
Scale up your PR contributions gradually in line with your ability and understanding of the project, process, and codebase.
Continue to do more reviewing and testing than adding PRs to the backlog. Reviewing 5 to 15 PRs for every PR you open is a good range.
If you are a good reviewer and begin building a reputation and appreciation for your help in moving the project forward, maintainers and other contributors may reciprocate by reviewing your work more quickly as well. See these two articles for more: