Skip to content

Commit c738487

Browse files
committed
pull-requests.md: add guidance for large and/or automatic edits
This primarily came out of the discussion around allowing the use of LLMs (kubernetes/steering#291), but isn't limited to it because other tools (search/replace, linters) can have the same effect. The goal is to clarify expected behavior and to give reviewers something that they can link to when they decide that a PR shouldn't get merged.
1 parent eeaf3e9 commit c738487

File tree

1 file changed

+33
-1
lines changed

1 file changed

+33
-1
lines changed

contributors/guide/pull-requests.md

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ It should serve as a reference for all contributors, and be useful especially to
3535
- [It's OK to Push Back](#its-ok-to-push-back)
3636
- [Common Sense and Courtesy](#common-sense-and-courtesy)
3737
- [Trivial Edits](#trivial-edits)
38+
- [Large or Automatic Edits](#large-or-automatic-edits)
39+
- [Fixing Linter Issues](#fixing-linter-issues)
3840
- [The Testing and Merge Workflow](#the-testing-and-merge-workflow)
3941
- [More About `Ok-To-Test`](#more-about-ok-to-test)
4042

@@ -268,13 +270,18 @@ handful of people who can approve changes across large portions of the repositor
268270
are generally the people who are the most busy and hardest to get reviews from, especially
269271
when you're a new contributor with no connections within the community yet.)
270272

273+
The effort required to review such sweeping changes might not be worth it, see
274+
["large or automatic edits")[#large-or-automatic-edits] below.
275+
271276
If you really want to try to get such a PR merged, your best bet is to break up the PR
272277
into separate PRs for each SIG whose code it touches. You can look at the `OWNERS` files
273278
in a directory (or its parent directory) to see who owns that code, and then group the
274279
changes together accordingly (e.g., with one PR touching files in `cmd/kube-proxy` and
275280
`pkg/util/iptables`, which are owned by SIG Network, and another PR touching files in
276281
`pkg/kubelet` and `pkg/controller/nodelifecycle`, which are owned by SIG Node.)
277282

283+
284+
278285
## Comments Matter
279286

280287
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.
@@ -590,7 +597,32 @@ at once to that file.
590597
* Can the file be improved further?
591598
* Does the trivial edit greatly improve the quality of the content?
592599

593-
## Fixing linter issues
600+
## Large or Automatic Edits
601+
602+
Some tools make it very easy to create large Pull Requests, for example:
603+
- global search/replace
604+
- linters which automatically correct issues (see also next section)
605+
- large language models (LLMs) which generate code or documentation
606+
607+
To make it easier for reviewers to handle such Pull Requests, please explain
608+
how it was generated in the "Special notes for your reviewer" section of the
609+
Pull Request description. Reviewers may then be able to reproduce those steps
610+
(search/replace, linters) or can start the review with the right expectations
611+
(LLMs). Also consider the section about [splitting up Pull
612+
Requests](#dont-open-pull-requests-that-span-the-whole-repository) above.
613+
614+
Even with such tools it is still your responsibility as submitter of a Pull
615+
Request to ensure that the change is correct (to the best of your knowledge),
616+
and that making the change improves the project enough to justify the cost that
617+
is needed to review and merge the Pull Request (see previous section). If
618+
unsure, create a Draft Pull Request and ask for guidance.
619+
620+
Please understand that reviewers may decide to close a Pull Request with a
621+
reference to this documentation if they come to the conclusion that the
622+
difficulty of properly reviewing the Pull Request outweighs the benefit that
623+
the Pull Request provides.
624+
625+
## Fixing Linter Issues
594626

595627
Kubernetes has a set of linter checks. Some of those must pass in the entire
596628
code base, some must pass in new or modified code, and some are merely hints

0 commit comments

Comments
 (0)