Last updated: 11 June 2020
On Reviewing, and Helping Those Who Do It

Reviewing is not only the most effective way to contribute as a new contributor to Bitcoin, it is also an important way to influence consensus on changes. Just as our daily choices and actions are the best way we can influence things, our review choices matter profoundly and cumulatively to Bitcoin.

The Current State of Affairs

Several hundred pull requests are open at any given moment in Bitcoin Core waiting for review and testing, and the stack continues to grow higher — 280 in November 2019, 380 at the end of April 2020, and now 400 at the start of June, when the number of PRs in the blockers list also reached a record high number.

Review is currently not keeping pace with PRs. The same is true for open issues, which generally keep growing in number, outside of occasional exemplary efforts to weed and close stale ones.

Valuable, high-priority PRs can be seen sitting for months or even years without enough reviewers, subjecting their authors to frustration, discouragement, and rebase hell until they abandon working on them.

This is not a criticism of the maintainers. They're doing a great job. They need help: more review and testing.

The number of people doing good, consistent review and testing is surprisingly low. On any given day, we see a handful of regular contributors reviewing on a daily basis. Of these, fewer still have the situation and predisposition to tackle review of the harder or larger PRs. That leaves us with very few consistent reviewers present most days.

When I describe this situation to people, even to established ones in the Bitcoin space, they tend to be astonished at how few people the system is depending on.

Current Issues

This state of affairs creates several problems:

  1. High-priority, larger, or harder-to-review PRs can go for months (and even years) without sufficient review.
  2. Regular PRs that provide good but non-urgent improvements or prevention can sometimes sit for months without review, or until an issue they address causes pain from a bug, regression, or vulnerability.
  3. Any PR of non-immediate urgency beyond a certain size, number of commits, or level of complexity is a candidate for needing to be separated into bite-sized PRs in order to attract reviewers, which multiplies the number of PRs.

    While small focused PRs are good, and separating large PRs into logical small ones can improve the code, some changes are better done in one go.
  4. Grease (review, merges) sometimes goes to the squeaky wheel. Such is life.
  5. PR authors may be incentivized to open more trivial PRs that have a better chance of being reviewed and merged within a reasonable time (until the author tires of making this sort of contribution and aspires to greater things, before being discouraged again).
  6. Things are often abandoned or never quite finished. Some contributors leave or take long periods of absence from the project out of frustration. Other people shake it off but feel recurring discouragement. Yes, it's open source. But I think we can improve the situation.
  7. Managing context, relevance, and being kind can make a big difference. Bitcoin Core code review is a social process. Getting your PRs reviewed without being too annoying (hopefully by helping others and doing consistent review yourself) can be an art.
  8. Nevertheless, the project and its reviewers run a continual risk of mal-investment — scarce review resources going to poorly done, annoying, insufficiently researched, or dangerous changes, or ones that add technical debt — rather than catching a regression or vulnerability, fixing issues, improving test coverage, finishing ongoing work, doing long-term work, and other priorities.
  9. Above all, not enough contributors do consistent review and follow the rule of thumb of 5 to 15 reviews or issues handled per PR opened.

What to Review

Given this state of affairs, which PRs we review with our limited time is an important choice, and one that might be under-appreciated.

The PRs we review represent:

  • what areas we contribute to
  • which project priorities matter to us
  • what kind of PRs we incentivize
  • ultimately, which behavior we reward

With this in mind, I try to pay more attention to selecting the best PRs to review. It's a juggling act, to be sure, and guilt-driven review is also a real (and valid) driver. Nevertheless, here are my criteria:

  • What: choosing harder, high-value PRs over new, easy or trivial ones (knowing that the latter will see review anyway) unless they are useful, interesting, or an area I know well.
  • Who: rewarding people who do consistent or outstanding review/issue testing, or who have been kind or patient. Some of the most deserving people haven't seen enough review from me yet, but I've noticed who you are and am working on it.
  • I also try to avoid reviewing or encouraging these kinds of PRs:
    1. PRs that are poorly done, annoying or unlikely to be merged.
    2. PRs that are unfinished or lacking sufficient research.
    3. PRs whose goals or approach I don't agree with or that seem dangerous (another option is to NACK as a last resort).
    4. PRs where a large amount of arguing is taking place; these PRs attract a lot of attention anyway.

On Reviewing

Sure, writing code and opening PRs is more fun, glorifying, and newsworthy. But it needs to be accompanied by a project-sustainable level of reviewing and testing or it becomes immature and short-sighted. A balance here seems important. Reviewing, issue testing, release candidate testing, and building gitian signatures are vital to Bitcoin and not optional to the health of the project. We need to reward selfless review and testing and helping others.

Hopefully the Bitcoin Core PR Review Club can help stimulate review, but I'm thinking about how else we can encourage it, add incentives, social norms, and so forth.

If you are a newer or less-experienced contributor, please don't let that be an excuse or discourage you from testing issues, release candidates, and reviewing. These activities will teach you more and be of higher value to the project — and to you — than just opening PRs.

Read through the recently-active PRs and issues and choose ones that are somewhat accessible — ideally right at the edge of your current reach — and ramp up gradually to the harder and important high-priority ones.

As Steve Lee of Square Crypto recently wrote: "If literally all a developer did was review hard but important PRs it'd be incredibly valuable." It would be wonderful if more open source contributors and sponsors/donors shared this view.

What To Do

This is open source; it's ad hoc and free-flowing. These ideas are proposed more as guidelines than as rules; as goals to think about and consider.

Contributing is not only about commits. It would be great to get away from the idea of telling people that contributing equals opening PRs. Contributing is much more than this and it could be reflected in what we say and do.

Continue to do more reviewing and issue testing than adding PRs to the backlog. Reviewing 5 to 15 PRs (or testing 5-15 issues) for every PR you open is a good range. Communicate that norm to people.

Before and after opening a PR in a particular domain, review the other open PRs in the same domain. Presumably you have or could use more domain expertise. Gather and share it by reviewing and testing the related PRs.

Participate every week in the Bitcoin Core PR Review Club, not only as a way to learn but primarily to actually do review. Reviewers, both beginners and experts, are the true heroes. Bitcoin needs them.

Find a way to reward people who are doing consistent review or testing. It's a harder and less-celebrated path. These people need support. Review their work. Help them. Donate Bitcoin to them.

Happy contributing!

Cheers,

Jon Atack