alamb commented on pull request #1935: URL: https://github.com/apache/arrow-datafusion/pull/1935#issuecomment-1064118121
> For work like this in the future (large refactorings broken into multiple PRs which depend on each other) it might be a good idea to do something similar to the arrow2 branch. That is, we can create a feature branch for the overall refactoring and raise the incremental work as PRs to the feature branch. It is a little more annoying to manage but it makes the individual PRs much easier to review. This basically approximates a "stacked diffs" (https://kurtisnusbaum.medium.com/stacked-diffs-keeping-phabricator-diffs-small-d9964f4dcfa6) model. I actually prefer to avoid large feature branches (like `arrow2` if possible) -- in my experience they are hard to maintain and collect conflicts quickly. Getting the changes into master as soon as possible I have found better over the long ter. I think the sequence of PRs from @yahoNanJing is actually a pretty good way (I think they are basically a stacked diff). I especially like keeping changes that "moving code around, but keeping the behavior the same" into their own PRs as I think they are easy to review and we can merge them quickly. In this case, I am sorry it has taken some time to get the in -- as you can see we were a little bottlenecked on review capacity with an influx of contributors. I hope this bottleneck is reduced over time The downside is that when you end up with 4-5 PRs chained together the overall goal sometimes is harder to see. I have found making a single tracking issue that explains the overall plan and lists the individual steps works well Like - [ ] Step 1: Refactor XX (link to PR) - [ ] Step 2: Refactor YY (link to PR) - [ ] Step 3: Refactor ZZ (link to PR) - [ ] Step 4: Change behavior (link to PR) ``` - [ ] Step 1: Refactor XX (link to PR) - [ ] Step 2: Refactor YY (link to PR) - [ ] Step 3: Refactor ZZ (link to PR) - [ ] Step 4: Change behavior (link to PR) ``` I don't think there is any reason to have a 1-to-1 correspondence between issue and PR if the PRs are chained together. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
