If you want to go fast, go alone. If you want to go far, go together.
As a community, we value going far over going fast., To go far, we need to stay aligned in our values, goals, and practices. One of the ways that we do this is through our culture and practice of reviews
Review culture came to us from the practices of software engineers. Large codebases are incredibly complex: they contain the ideas and work of hundreds of people, manifested in millions of lines of code. They need to remain logically consistent, even as they grow and change. A big part of how engineers manage is through review: every single change to the system is reviewed by at least one other person before becoming official, or "merging".
Review culture brings many benefits.
First, it leads to higher quality output, because the version that merges will have benefited from a second pair of eyes, and feedback.
Even more importantly, review enables the exchange of knowledge and context. Through discussing the change under review, both the contributor and the reviewer can teach each other about the goals, techniques, and practices of the project.
The review process orients around changes, which are the units of work being reviewed. A change can take many forms, for example:
- Adding a new guide or piece of documentation
- Adding a new module to the codebase
- Modifying an existing guide or module
The contributor is the person proposing a change. Anyone can decide to be a contributor: the only requirements are that you have a change you need to propose, and that you agree to be a respectful participant in the review process. (When we have a code of conduct--link to it here.)
The reviewers are the people who review the change, suggest changes to it, and eventually approve it. Anyone in the community can act as a reviewer: the only requirements are that you have some insight to share (or questions to ask) about the change, and that you agree to be a respectful participant in the process.
The maintainers are responsible for the health and maintenance of the project, and have final say in the review process. Maintainers can block changes they think are harmful, or approve a change over others' objections. Someone can be a maintainer within a specific domain: for example, acting as maintainer for a particular repository, or even maintaining a particular guide. Since maintainers have special powers, they also have a special responsibility to act thoughtfully and fairly. New maintainers are chosen by the existing maintainers. In general, people who have a long track record of acting as contributors and reviewers may be good candidates for becoming maintainers.
The first step is for a contributor to propose a change. The contributor needs to do two things at this stage:
- Flesh out the change (e.g. writing the code), so that there is something to review
- Explain the change (e.g. writing a pull request description), so that reviewers can easily understand it
Both steps are vital, and contributors should put real effort into explaining the change well. Its not fair to ask a reviewer to puzzle through an undocumented change.
Finally, before requesting a review, the contributor should self-review their change. It's amazing how many issues can be spotted and fixed during self-review. This is respectful of reviewers' time, and makes the process a lot smoother.
Once a change is ready and has been self-reviewed, it's ready for review. Since most SourceCred artifacts are tracked via repositories on GitHub, this will usually mean creating a pull request.
Once the change is proposed, anyone in the project can add their own review. The person proposing the change can also request specific reviewers, if certain individiuals have context or knowledge thet will make their reivew particularly valuable.
Reviewers should focus their reviews on substantial, rather than stylistic, matters. This requires some effort, since it's usually easier to review for style rather than substance.
A review that suggests changing a bunch of specific words, or re-naming variables, is not nearly as useful as a review which explores other ways the problem could be solved, or the core assumptions that went into the change.
On finishing their review, the reviewer can:
- ask questions to better understand the change
- suggest changes to it
- approve the change
Most changes go through a few rounds of reviews before being approved.
During the review process, a successful change will accumulate "approvals" from satisfied reviewers. At some point, the change will have "enough approvals" and will be ready to merge. There's no hard-and-fast rule to determine when a change has enough approvals. Generally, simple and small changes are easy to approve, whereas changes that are breaking new ground, or have wide-ranging consequences, will require more reviews. If you're unsure as to whether a change is ready to merge, ask one of the maintainers.