Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Run clang-tidy checks in PR CI #17407

Merged
merged 14 commits into from
Nov 22, 2024
Merged

Run clang-tidy checks in PR CI #17407

merged 14 commits into from
Nov 22, 2024

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Nov 21, 2024 •
edited
Loading

Description

We discussed clang-tidy during the cuDF brown bag session. We decided to enable clang-tidy in PR CI and follow up by enabling more checks later.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 21, 2024
@github-actions github-actions bot removed the libcudf Affects libcudf (C++/CUDA) code. label Nov 21, 2024
@bdice bdice changed the title Expand clang-tidy checks Run clang-tidy checks in PR CI Nov 21, 2024
@bdice bdice marked this pull request as ready for review November 21, 2024 22:47
@bdice bdice requested a review from a team as a code owner November 21, 2024 22:47
@bdice bdice requested a review from KyleFromNVIDIA November 21, 2024 22:47
@bdice bdice mentioned this pull request Nov 21, 2024
Open
21 tasks
@bdice bdice requested a review from a team as a code owner November 21, 2024 23:15
@bdice bdice requested review from harrism and lamarrr November 21, 2024 23:15
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Nov 21, 2024
@bdice
Copy link
Contributor Author

bdice commented Nov 21, 2024

This is ready for review.

vyasr
vyasr reviewed Nov 21, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I suggest that we remove this job from test.yaml since we don't need to run linters in nightlies.

@vyasr vyasr requested a review from a team as a code owner November 22, 2024 01:03
@github-actions github-actions bot added the CMake CMake build issue label Nov 22, 2024
@vyasr
Copy link
Contributor

vyasr commented Nov 22, 2024

We realized that include-what-you-use is causing some of the compiler issues, so I've added back the nightly job to run IWYU while only clang-tidy is run in CI. As IWYU matures we may eventually be able to change that.

vyasr
vyasr approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I converted warnings back into errors for clang-tidy since we have a clean run now.

I'm approving, but we should probably get at least one other reviewer since I've written the bulk of this PR now.

davidwendt
davidwendt approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

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

Approving C++ changes.

bdice
bdice commented Nov 22, 2024
View reviewed changes
.github/workflows/pr.yaml Outdated Show resolved Hide resolved
bdice
bdice commented Nov 22, 2024
View reviewed changes
ci/cpp_linters.sh Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor

vyasr commented Nov 22, 2024

/merge

@rapids-bot rapids-bot bot merged commit 5a89d00 into rapidsai:branch-25.02 Nov 22, 2024
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@davidwendt davidwendt davidwendt approved these changes

@KyleFromNVIDIA KyleFromNVIDIA Awaiting requested review from KyleFromNVIDIA KyleFromNVIDIA is a code owner automatically assigned from rapidsai/ci-codeowners

@harrism harrism Awaiting requested review from harrism harrism is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

@lamarrr lamarrr Awaiting requested review from lamarrr lamarrr is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@vyasr vyasr

@bdice bdice

Labels
2 - In Progress Currently a work in progress CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bdice @vyasr @davidwendt