+1 (non-binding) LGTM in Parquet part
Best, Xuwei Fu Sutou Kouhei <k...@clear-code.com> 于2024年8月28日周三 09:07写道： > Hi, > > How about indenting preprocessor directives for readability? > > Issue: https://github.com/apache/arrow/issues/43796 > PR: https://github.com/apache/arrow/pull/43798 > > For example: > > Before: > > ---- > #ifndef NDEBUG > #include "arrow/acero/options_internal.h" > #endif > ---- > > After: > > ---- > #ifndef NDEBUG > # include "arrow/acero/options_internal.h" > #endif > ---- > > > FYI: Google C++ style guide (our coding style is based on > it) doesn't require indent in preprocessor directives nor > deny it: > > https://google.github.io/styleguide/cppguide.html#Preprocessor_Directives > > ---- > // Good - directives at beginning of line > if (lopsided_score) { > #if DISASTER_PENDING // Correct -- Starts at beginning of line > DropEverything(); > # if NOTIFY // OK but not required -- Spaces after # > NotifyClient(); > # endif > #endif > BackToNormal(); > } > ---- > > > Thanks, > -- > kou >
