Skip to content

Commit c85d66e

Browse files
committed
Add a section to the pull-request docs about giant multi-SIG PRs
1 parent 8c26d6a commit c85d66e

File tree

1 file changed

+26
-0
lines changed

1 file changed

+26
-0
lines changed

contributors/guide/pull-requests.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ It should serve as a reference for all contributors, and be useful especially to
2626
- [Is the feature wanted? File a Kubernetes Enhancement Proposal](#is-the-feature-wanted-file-a-kubernetes-enhancement-proposal)
2727
- [Smaller Is Better: Small Commits, Small Pull Requests](#smaller-is-better-small-commits-small-pull-requests)
2828
- [Open a Different Pull Request for Fixes and Generic Features](#open-a-different-pull-request-for-fixes-and-generic-features)
29+
- [Don't Open Pull Requests That Span the Whole Repository](#dont-open-pull-requests-that-span-the-whole-repository)
2930
- [Comments Matter](#comments-matter)
3031
- [Test](#test)
3132
- [Squashing](#squashing)
@@ -249,6 +250,31 @@ Likewise, if Feature-X is similar in form to Feature-W which was checked in last
249250
Feature-X.
250251
(Do that in its own commit or pull request, please.)
251252

253+
## Don't Open Pull Requests That Span the Whole Repository
254+
255+
Often a new contributor will find some problem that exists in many places across the main
256+
`kubernetes/kubernetes` repository, and file a PR to fix it everywhere at once. Maybe
257+
there's a cool new function in the latest golang release that everyone ought to be using,
258+
or a recently-deprecated function that ought to be replaced with calls to its replacement.
259+
Sometimes a contributor will run a linter or security scanner across the code to find
260+
problems, or fix a particular spelling mistake in comments or variable names. (It's
261+
"deprecated", not "depreciated"!)
262+
263+
The problem with this approach is that different parts of `kubernetes/kubernetes` are
264+
maintained by different SIGs, and so changes to those different parts require approvals
265+
from different people. A PR containing 20 one-line changes scattered across the repository
266+
could end up needing 5 or 10 approvals or more before it can be merged. (While there are a
267+
handful of people who can approve changes across large portions of the repository, those
268+
are generally the people who are the most busy and hardest to get reviews from, especially
269+
when you're a new contributor with no connections within the community yet.)
270+
271+
If you really want to try to get such a PR merged, your best bet is to break up the PR
272+
into separate PRs for each SIG whose code it touches. You can look at the `OWNERS` files
273+
in a directory (or its parent directory) to see who owns that code, and then group the
274+
changes together accordingly (e.g., with one PR touching files in `cmd/kube-proxy` and
275+
`pkg/util/iptables`, which are owned by SIG Network, and another PR touching files in
276+
`pkg/kubelet` and `pkg/controller/nodelifecycle`, which are owned by SIG Node.)
277+
252278
## Comments Matter
253279

254280
In your code, if someone might not understand why you did something (or you won't remember why later), comment it. Many code-review comments are about this exact issue.

0 commit comments

Comments
 (0)