Skip to content

Enable targeted clang-tidy readability checks - #317

Merged
scal444 merged 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:codex/clang-readability
Sep 22, 2026
Merged

scal444 merged 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:codex/clang-readability

Conversation

@scal444

@scal444 scal444 commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

Enable a focused set of low-churn clang-tidy readability checks and apply their fixes to project sources.

@greptile-apps

greptile-apps Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because the latest configuration expansion introduces no established build failure or behavioral regression.

Summary

This PR expands the project’s clang-tidy policy with focused readability checks, applies the corresponding mechanical fixes across C++ sources, and scopes analysis to project sources.

  • Adds selected readability checks and a curated cppcoreguidelines configuration.
  • Updates the clang-tidy runner’s source and header filters.
  • Applies brace, container API, pointer qualification, and redundant-cast cleanups without changing intended runtime behavior.
  • The previous checkout-path regex concern was manually resolved and is not outstanding.

Reviews (4) · Last reviewed commit: "Merge branch 'main' into codex/clang-rea..."

Comment thread admin/run_clang_tidy.sh
@scal444
scal444 requested a review from evasnow1992 September 19, 2026 14:05
@scal444

scal444 commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator Author

The one I care about most here is the statement braces, since the models love to do one line indented statements that look awful. #308

Comment thread src/data_structures/flat_bit_vect.h Outdated

@evasnow1992 evasnow1992 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changes look good to me. Just one comment regarding the removal of typename, which may cause a compilation error with Clang 15. Or we may want to update the readme file to recommend a different compiler/version?

@scal444
scal444 merged commit 0765893 into NVIDIA-BioNeMo:main Sep 22, 2026
15 checks passed
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