I've definitely been on both sides: posting a large PR myself, and as a reviewer asking for PRs to be separated into smaller pieces. If we want something persisted beyond an email thread I think we could update CONTRIBUTING.md with a new section about how to post larger changes. Tips for breaking down PRs that we could document: - Separating refactoring PRs from the new change that depends on the refactoring - Splitting one refactoring of a general area of code into individual refactoring PRs to iterate from the current state to the desired state. - Use parent jiras with subtasks as part of planning, before coding. - Provide design docs or at least detailed PR descriptions to make complex reviews more manageable. - A large PR with a description covering multiple disjoint issues may be better split to independent fixes.
I mention refactoring a lot because those sorts of PRs are frequent culprits of these large diffs. One example I had recently was refactoring in https://github.com/apache/ozone/pull/4838 followed by the change dependent on the refactoring in https://github.com/apache/ozone/pull/4867. The latter was still a large PRs but splitting helped speed up review and keep the diff under 1k lines. Made my dev work easier too. I don't think a large PR should warrant a CI failure either, this seems like the type of thing that would be up to reviewers to enforce since it impacts them the most. If you're set to review a large change, ask if there is any way it can be split to a dependency chain of two or more consecutive PRs. Having something written in COMTRIBUTING.md will provide a source that reviewers can point devs towards as a reference in these situations. Ethan On Wed, Jul 26, 2023 at 3:02 PM Wei-Chiu Chuang <weic...@apache.org> wrote: > Hi Ozoners, > > In one of the coffee break chats with a colleague of mine, we realized many > of the PRs in the Ozone project are quite lengthy. > > I'm guilty of this myself too. Keeping PR short and sweet is good hygiene. > It allows reviewers to spot potential problems in the code easier, and your > PR is more likely to be reviewed and iterated quickly. > > How would you like to see the PR quality improved? I'd like to urge > everyone to break down PRs but I don't necessarily want a GitHub Action > that enforces length limit. :) > > Weichiu >
