yahoNanJing commented on pull request #1912: URL: https://github.com/apache/arrow-datafusion/pull/1912#issuecomment-1060193956
> Thanks @yahoNanJing -- I skimmed this PR and the basic idea of breaking the system down into smaller modules I think is a great direction. > > One thing I wanted to mention that might help with review / merge speed in the future is to break such refactoring PRs into two parts: > > 1. A PR that just moves code around / breaks up files but doesn't change behavior > 2. The PR that changes behavior > > The reason is that often moving files around causes large diffs in github but if there is no change in behavior can be straightforward to review / merge > > The changes in behavior may take some more thought / comment and can be harder to understand in a larger diff. > > Also, a larger PR has a greater chance to accumulate conflicts so if behavior changes get delayed in review, that can also cause issues Thanks @alamb. The background of doing this refactoring is mainly for achieving the final goal of #1704. Without these refactoring, all of the complex things will messed up in one or two files, which is really hard for maintenance. For the step by step refactoring, every PR will be tested and verified without changing the behavior. -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
