Skip to content

[CI] Enable clang-tidy in precommit#21234

Open
KornevNikita wants to merge 1 commit intointel:syclfrom
KornevNikita:clang-tidy
Open

[CI] Enable clang-tidy in precommit#21234
KornevNikita wants to merge 1 commit intointel:syclfrom
KornevNikita:clang-tidy

Conversation

@KornevNikita
Copy link
Contributor

I'm not sure if clang-tidy passes on all PRs, so for now introducing with continue-on-error (meaning it will be green even in case of failure).

I'm not sure if clang-tidy passes on all PRs, so for now introducing
with continue-on-error (meaning it will be green even in case of
failure).
@KornevNikita KornevNikita requested a review from a team as a code owner February 6, 2026 17:40
run-clang-tidy:
runs-on: [Linux, build]
container:
image: ghcr.io/intel/llvm/sycl_ubuntu2404_nightly:latest

Check failure

Code scanning / zizmor

unpinned image references Error

unpinned image references
Comment on lines +15 to +19
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
with:
sparse-checkout: |
devops/actions
devops/scripts

Check warning

Code scanning / zizmor

credential persistence through GitHub Actions artifacts Warning

credential persistence through GitHub Actions artifacts
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool change, just a few comments


# Remove gcc-specific flags
perl -0777 -pe '
s/(?:^|[ \t])\-Wno-class-memaccess(?=$|[ \t])//g;
Copy link
Contributor

@sarnex sarnex Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this could we just use clang, even just some system package manager installed one, to run configure?

from typing import Iterator

EXCLUDE_PATH_RE = re.compile(r"(?:^|/)(test|test-e2e|unittests)(?:/|$)")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: two lines inbetween instead of one in a few places

if: steps.should_run_tidy.outputs.run == 'true'
run: |
# Remove commands containing "-D__INTEL_PREVIEW_BREAKING_CHANGES" to avoid running on the same file twice.
jq '[ .[] | select(.command | contains("-D__INTEL_PREVIEW_BREAKING_CHANGES") | not) ]' $GITHUB_WORKSPACE/build/compile_commands.json > $GITHUB_WORKSPACE/build/compile_commands.temp.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just pass -DSYCL_ENABLE_MAJOR_RELEASE_PREVIEW_LIB=Off instead of doing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants