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).
When a new contributor is doing review or testing, they stand out positively and much more than someone who is opening pull requests and doing little to no commensurate review and testing.
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.
Read and know the Bitcoin Core Contributing Guide, Developer Notes and Productivity Notes.
What to do
Choose your contribution carefully to be sure it is desired by review; 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:
- the Contribute page
- the "good first issue" list
Good ideas may also be found while doing PR reviews and in #bitcoin-core-dev IRC discussions when experienced contributors mention nice-to-haves.
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
run git grep "TODO\|FIXME" on the command line from
root. Keep in mind that these comments are often out of date.
Priorities
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?
Try to solve a clearly defined issue with a clear, minimal change.
Avoid making trivial refactoring, style cleanup, or spelling PRs as a newcomer; these consume valuable contributor 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. As described in CONTRIBUTING.md, it's best to leave refactoring to the experienced contributors.
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 information
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 git
log -S history. Look at the git blame of the
code in question and git show / git
log the commits of the relevant lines. Check the GitHub
file history. You'll see who understands that part of the
codebase and can be contacted for questions or review (after
you've first put in the work to understand it on your own).
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. That said, I do it less and less, and now quite rarely, because observing discussion on IRC and while reviewing pull requests allows seeing needs naturally.
Write test coverage
Test coverage is essential; don't hesitate to write any missing unit, functional, or fuzz tests, or improve the benchmarks.
Write documentation
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
Doxygen documentation,
test logging (both info and debug),
and so on.
Add
release notes
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
of getbalances for details."
To avoid potential merge conflicts, release notes are sometimes
added in a separate markdown file in the /doc
directory named release-notes-<PR_NUMBER>.md,
not directly in
doc/release-notes.md.
In general, PRs that intelligently improve documentation and tests in a well thought-out way, after some time invested in observing and reviewing to understand what is desired, tend to be well-received.
Hygienic commits
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 --enable-debug
passed to your build configuration. Run
./configure --help | grep -A1 --enable-debug
for more info.
Set up a CI (continuous integration) on your own GitHub Bitcoin repository, so that when you push a branch or commit, the linter and CI tests run. It can be a good idea to verify that they are all green before opening a PR. The CI is 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.
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 open.
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, and a logical ordering of commits that each build cleanly and pass all tests.
Timing
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 not exceeding that unless the quality and quantity of your reviews (or handling and testing of issues) are also present in equal or greater proportion.
Give some thought to PR size and scope. Sometimes, 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.
PR descriptions
As described in the Bitcoin Core
contributing guide,
prefix your PR title with the relevant area
(e.g. doc, test, rpc, wallet).
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.
Don't 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 as text, and any formatting will be lost.
Say why
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".
Rebasing
It's 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 a contributor 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.
Getting review
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: Make sure your work is interesting, desired, and done well. Give back by helping others and giving review. Give more than you take or ask for; add value as best as you can while figuring things out on your own. Be humble, kind and forgiving. 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 be merged more quickly.
Scale up your PR contributions gradually in line with your ability and understanding of the project, process, and codebase.
Keep reviewing
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, consistent reviewer and begin building a reputation and appreciation for your help in moving the project forward, other contributors may reciprocate by reviewing your work more quickly as well. See these two articles for more:
Happy contributing!
Cheers,
Jon Atack