diff --git a/contributors/guide/pull-requests.md b/contributors/guide/pull-requests.md index be65671e939..a657337381d 100644 --- a/contributors/guide/pull-requests.md +++ b/contributors/guide/pull-requests.md @@ -35,6 +35,8 @@ It should serve as a reference for all contributors, and be useful especially to - [It's OK to Push Back](#its-ok-to-push-back) - [Common Sense and Courtesy](#common-sense-and-courtesy) - [Trivial Edits](#trivial-edits) + - [Large or Automatic Edits](#large-or-automatic-edits) + - [Fixing Linter Issues](#fixing-linter-issues) - [The Testing and Merge Workflow](#the-testing-and-merge-workflow) - [More About `Ok-To-Test`](#more-about-ok-to-test) @@ -268,6 +270,9 @@ handful of people who can approve changes across large portions of the repositor are generally the people who are the most busy and hardest to get reviews from, especially when you're a new contributor with no connections within the community yet.) +The effort required to review such sweeping changes might not be worth it, see +["large or automatic edits")[#large-or-automatic-edits] below. + If you really want to try to get such a PR merged, your best bet is to break up the PR into separate PRs for each SIG whose code it touches. You can look at the `OWNERS` files in a directory (or its parent directory) to see who owns that code, and then group the @@ -275,6 +280,8 @@ changes together accordingly (e.g., with one PR touching files in `cmd/kube-prox `pkg/util/iptables`, which are owned by SIG Network, and another PR touching files in `pkg/kubelet` and `pkg/controller/nodelifecycle`, which are owned by SIG Node.) + + ## Comments Matter 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. * Can the file be improved further? * Does the trivial edit greatly improve the quality of the content? -## Fixing linter issues +## Large or Automatic Edits + +Some tools make it very easy to create large Pull Requests, for example: +- global search/replace +- linters which automatically correct issues (see also next section) +- large language models (LLMs) which generate code or documentation + +To make it easier for reviewers to handle such Pull Requests, please explain +how it was generated in the "Special notes for your reviewer" section of the +Pull Request description. Reviewers may then be able to reproduce those steps +(search/replace, linters) or can start the review with the right expectations +(LLMs). Also consider the section about [splitting up Pull +Requests](#dont-open-pull-requests-that-span-the-whole-repository) above. + +Even with such tools it is still your responsibility as submitter of a Pull +Request to ensure that the change is correct (to the best of your knowledge), +and that making the change improves the project enough to justify the cost that +is needed to review and merge the Pull Request (see previous section). If +unsure, create a Draft Pull Request and ask for guidance. + +Please understand that reviewers may decide to close a Pull Request with a +reference to this documentation if they come to the conclusion that the +difficulty of properly reviewing the Pull Request outweighs the benefit that +the Pull Request provides. + +## Fixing Linter Issues Kubernetes has a set of linter checks. Some of those must pass in the entire code base, some must pass in new or modified code, and some are merely hints