wgtmac commented on code in PR #838:
URL: https://github.com/apache/iceberg-cpp/pull/838#discussion_r3637158441
##########
.github/workflows/cpp-linter.yml:
##########
@@ -91,24 +96,40 @@ 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 --upgrade pip
+ python -m pip install "cpp-linter==${CPP_LINTER_VERSION}"
"clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"
Review Comment:
Do we need clang-format in this job at all? The pre-commit workflow already
runs the pinned clang-format hook over all files on every PR. Keeping
formatting there and making cpp-linter tidy-only would avoid duplicate checks
and the version-sync problem. Could we drop `CLANG_FORMAT_VERSION` and the
`clang-format` package here, and set `--style` to an empty string to disable
format checks?
##########
.github/workflows/cpp-linter.yml:
##########
@@ -91,24 +96,40 @@ 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 --upgrade pip
+ python -m pip install "cpp-linter==${CPP_LINTER_VERSION}"
"clang-format==${CLANG_FORMAT_VERSION}" "clang-tidy==${CLANG_TIDY_VERSION}"
+ - name: Run cpp-linter
id: linter
if: github.event_name == 'pull_request'
continue-on-error: true
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- with:
- style: file
- tidy-checks: ''
- version: 22
- files-changed-only: true
- lines-changed-only: true
- thread-comments: true
- ignore: 'build|cmake_modules|ci|src/iceberg/catalog/hive/gen-cpp'
- database: build
- verbosity: 'debug'
- # need '-fno-builtin-std-forward_like', see
https://github.com/llvm/llvm-project/issues/101614
- extra-args: '-std=c++23 -I$PWD/src -I$PWD/build/src
-I$PWD/build/_deps/sqlpp23-src/include -I/usr/include/postgresql
-I/usr/include/mysql -fno-builtin-std-forward_like'
+ # need '-fno-builtin-std-forward_like', see
https://github.com/llvm/llvm-project/issues/101614
+ run: |
+ cpp-linter \
+ --style=file \
+ --tidy-checks='' \
+ --version="${CLANG_FORMAT_VERSION%%.*}" \
Review Comment:
Even with a tidy-only setup, deriving `--version=22` would still bypass the
pip pin. cpp-linter looks for `clang-tidy-22` first, and the probe run selected
`/usr/bin/clang-tidy-22` at 22.1.2 instead of the installed 22.1.8. Could we
point this to `${pythonLocation}/bin`, or omit `--version`, so the
pip-installed binary is actually used?
--
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]
