Actually I'd prefer adding a reminder in the PR template here https://github.com/apache/ozone/blob/master/.github/pull_request_template.md Seems like a good way to soft-enforce the rule.
On Wed, Jul 26, 2023 at 3:53 PM Ethan Rose <er...@cloudera.com.invalid> wrote: > 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 > > >
