[GitHub] [incubator-doris] gaodayue edited a comment on issue #2792: Reformat BE code 1/7

[GitBox <gi...@apache.org>]

gaodayue edited a comment on issue #2792: Reformat BE code 1/7
URL: https://github.com/apache/incubator-doris/pull/2792#issuecomment-575915489
 
 
   @morningman I understand your concern about such big code changes may cause too much merge conflicts for under-development PR. To some extent it is inevitable, but I'm trying my best to reduce the probability of conflicts or at least make resolving such conflicts easy by
   
   1. Trying to make code changes happen on small discrete statements instead of large blocks. For example, if a file is using the old 2 space indent so that every single lines will be changed, reformat is completely disabled for that file.
   
   2. Dividing the whole reformat work into several independent PRs such that it is reviewable. If any reviewer find that there are too many code changes for a file or module which will cause it hard to rebase some PR, we can disable reformat on that particular file/module.
   
   > How about just leave the old code as what it was,
   and find a way to make sure that new code will obey the code style?
   
   Clang-format works on individual file so there are two ways to fix code style
   
   1. Check and fix code style only for changed files in each PR. This has the benefit of do the work incrementally. However, whenever a PR modifies a file with style issue, the PR will contain reformat changes along with logic changes. This will make review such PR laborious.
   
   2. Do one-time reformat on all files. This has the benefit that future PR doesn't need to reformat existing files and can focus on the logic. The downside is also obvious, you already mentioned a few.
   
   To my knowledge many projects choose the second approach. Maybe we can be more flexible such as trying one-time reformat on certain modules to start with, but leave some modules unmodified? 
   
   Hope to here your suggestion  :)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org