LuciferYang commented on code in PR #838:
URL: https://github.com/apache/iceberg-cpp/pull/838#discussion_r3637088139
##########
.github/workflows/cpp-linter.yml:
##########
@@ -91,24 +91,38 @@ jobs:
with:
path: ${{ github.workspace }}/.sccache
key: sccache-cpp-linter-ubuntu-${{ github.run_id }}
- - uses:
cpp-linter/cpp-linter-action@0f6d1b8d7e38b584cbee606eb23d850c217d54f8 # v2.15.1
+ - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 #
v6.3.0
+ if: github.event_name == 'pull_request'
+ with:
+ python-version: '3.13'
+ - name: Install cpp-linter and clang tools
+ if: github.event_name == 'pull_request'
+ run: python -m pip install "cpp-linter==1.13.0" "clang-format==22.1.8"
"clang-tidy==22.1.8"
Review Comment:
Before we settle on 22.1.5, one thing I hit while testing the pin. Running
clang-format 22.1.5 (the version pre-commit pins) over the current tree, 4
files come back as non-conforming: `catalog/rest/error_handlers.cc`,
`error_handlers.h`, `test/error_handlers_test.cc`, and
`util/error_collector.h`. They pass under 22.1.8 (what CI runs today), but
22.1.5 wants to reformat them.
So the tree is effectively formatted to 22.1.8 right now, while pre-commit
declares 22.1.5. The two are already out of sync. If CI's clang-format goes to
22.1.5, any later PR touching those files would fail the linter on formatting
its author never changed.
I'd lean toward putting both CI and pre-commit on 22.1.8 instead:
`CLANG_FORMAT_VERSION`, `CLANG_TIDY_VERSION`, and pre-commit all land on one
version, and nothing needs reformatting. Same "keep them in sync" outcome you
asked for, without the churn. WDYT?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
