-
Notifications
You must be signed in to change notification settings - Fork 49
In Praise of Small Pull Requests (PRs)
We strongly encourage developers contributing to POCS (and the other PANOPTES repositories) to create small, focused PRs. If you are developing a large feature, this can be challenging, yet everybody will benefit: you the author, the reviewer(s) and those merging your changes into their repos.
Before explaining why, here is a summary of what we'd like to be true:
As PR Writer I Will:
- Keep PRs Small
- Write PRs That Do Only One Thing
- Use Labels to Indicate if a PR is One of Many Parts
As PR reviewer I Will:
- Make an Effort to Review as Soon as I Get a Second
- Approve, as Long as it's Better than Before
- Prefer Creating an Issue or Requesting a Follow-Up PR to Make it Better, Over Rejecting the PR
- Favor Suggestions Over Rejections, Particularly when Multi-Part is Indicated via a Label
It is annoying after you've labored hard on a feature or fix to find that it doesn't get reviewed for hours, days or weeks. On the other hand, when you are a reviewer, it is daunting to find a PR to review with many files and hundreds or thousands of changed lines in it.
We can help each other by writing focused changes, that accomplish one, clear goal (e.g. fix a specific bug, add a single feature that can't reasonably be broken into sub-features). And as reviewers, we can help by reviewing promptly, making suggestions rather than demands, and approving if the change is positive (e.g. fixes a bug without introducing another). Feel free to suggest that names be improved, for example, but approve a good change, trusting that the author will make that change... until they demonstrate they can't be trusted to take the hint.
For larger changes that will span multiple PRs, please make use of GitHub's task list feature, where an issue's text can include a list with checkboxes (as opposed to numbers or bullets). This allows you to document your plan of work, to get feedback on that plan, and to record your progress implementing the plan.
See here for more information.