ORC-1307: Add .clang-format to enforce C++ code style

[wgtmac]

There are some guidelines on formatting: https://orc.apache.org/develop/coding/. But no tooling has been available to enforce the style of C++ code. This patch aims to be the starting point. Follow-up changes will include automatic scripts to format cpp source files and per-commit check.

[dongjoon-hyun]

Thank you so much, @wgtmac . I have three comments.

• This looks worth to have a JIRA ID. Could you file one?
• The code change should be in the same PR because we cannot merge this if this rule causes too breaking changes.
• In addition, we had better the CI enforcement of this rule in this PR together.

[dongjoon-hyun]

cc @williamhyun and @stiga-huang

[wgtmac]

I have tried to apply clang-format-action to verify whether the C++ files follow the .clang-format config file. However, I got an error as below:

jidicula/[email protected] is not allowed to be used in apache/orc. Actions in this workflow must be: within a repository owned by apache, created by GitHub, verified in the GitHub Marketplace, or matching the following: /@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+, AdoptOpenJDK/install-jdk@, JamesIves/github-pages-deploy-action@5dc1d5a, TobKed/label-when-approved-action@, actions-cool/issues-helper@, actions-rs/, al-cheb/configure-pagefile-action@, amannn/action-semantic-pull-request@, apache/, burrunan/gradle-cache-action@, bytedeco/javacpp-presets/.github/actions/, chromaui/action@, codecov/codecov-action@, conda-incubator/setup-miniconda@, container-tools/kind-action@, container-tools/microshift-action@, dawidd6/action-download-artifact@, delaguardo/setup-graalvm@, docker://jekyll/jekyll:, docker://pandoc/core:2.9, eps1lon/actions-label-merge-conflict@, gaurav-nelson/github-action-markdown-link-check@*,...

After some research, I figured out that this action is not whitelisted by the Apache project setting: https://github.com/orgs/community/discussions/25488. Unfortunately I don't have the permission to add it. Do you know how to do that, or is there any alternative? @dongjoon-hyun @williamhyun @stiga-huang

[dongjoon-hyun]

• Given that there are many other projects like Apache Arrow, there might be ASF-approved GitHub Action.
• We need to file an INFRA JIRA to ask about what is allowed or to ask to add a new GitHub Action plugin.

[wgtmac]

• Given that there are many other projects like Apache Arrow, there might be ASF-approved GitHub Action.
• We need to file an INFRA JIRA to ask about what is allowed or to ask to add a new GitHub Action plugin.

I have checked Apache Arrow but it uses cpplint manually and does not integrate it with the GitHub workflow.

jidicula/clang-format-action

I have checked apache/arrow, apache/avro, and apache/impala. It looks like none of them use Github workflow to apply style check. Apache Arrow uses cpplint manually but it requires hard work to match the style between clang-format and cpplint.

I have filed an INFRA JIRA: https://issues.apache.org/jira/browse/INFRA-23873.

[wgtmac]

@dongjoon-hyun Now it is good to go.

[dongjoon-hyun]

Thank you so much, @wgtmac .

[dongjoon-hyun]

+1, LGTM.

[dongjoon-hyun]

Since it looks very intrusive, cc @williamhyun , @omalley , @stiga-huang , @guiyanakuang , too.

[williamhyun]

+1 LGTM!

[guiyanakuang]

+1 LGTM

[stiga-huang]

Thanks for adding this! One concern is that this PR touch most of the c++ codes. git blame will always point to this PR.
It seems that keeping the indent inside namespace can save many changes. Probably we can have a try?

[wgtmac]

Thanks for adding this! One concern is that this PR touch most of the c++ codes. git blame will always point to this PR. It seems that keeping the indent inside namespace can save many changes. Probably we can have a try?

That's a good catch! Let me add NamespaceIndentation:All to reduce lines of change.

[stiga-huang]

Thanks for adding the new rule! Now I see some changes are refactoring simple functions like

    bool operator != (const FileVersion & right) const {
      return !(*this == right);
    }

into in one line

    bool operator!=(const FileVersion& right) const { return !(*this == right); }

Does it mean the previous syntax won't be accepted anymore? Impala allows both format. Not sure if this takes effect:

https://github.com/apache/impala/blob/3938dae8be6f28132de67f10d0361665e5c6cad3/.clang-format#L7

It'd be nice if both formats are allowed. But I'm ok with merging the current PR.

[wgtmac]

AllowShortFunctionsOnASingleLine: Inline

Unfortunately AllowShortFunctionsOnASingleLine: Inline fails the format check. I changed it to AllowShortFunctionsOnASingleLine: Empty instead. Please check @stiga-huang

[stiga-huang]

+1. Thanks for making the changes!

[wgtmac]

I just committed this. Thank you all! @dongjoon-hyun @stiga-huang @williamhyun @guiyanakuang