update to clang-tidy + workflow improvement cpp build & lint check #130

Merged

zach merged 14 commits from clang-tidy-fixes into main 2026-06-29 22:10:27 +02:00

Conversation 6 Commits 14 Files Changed 4

+104 -19

AramJonghu commented 2026-06-29 00:08:05 +02:00

Collaborator

Copy Link

Copy Source

Goal

Idea is to reduce the annoyances and research which clang-tidy rules make sense and actually have benefits in our project.

Also considering immediately resolving most if not all tidy warnings.

Current Changes

• .clang-tidy has changes to follow the guidelines we have been following silently. Some rules were different from how we program.
  • Note that naming is completely removed and won't be checked. In a future PR (where we go over the code to fix linting issues), we can reintroduce them and make sure they fit our codebase consistently.
• tidy rules are not considered errors anymore (stopped compile).
• CI/CD addition to build check and clang-tidy check. Tried to get clang-tidy check to show when failed, but that causes complications in its checks; so now it is simply silent when failing.
• Dockerfile that creates an arch container as cache for the CI/CD, prevents redownload of dependencies each run. Instead, it runs weekly (or optionally a different interval).

Current complications

• To push the docker container build, it would require this webserver to allow larger filesize pushes (need ~2GB). Current workaround is arguably better where docker build is pushed to runner server.
• Figuring out which clang-tidy rules make sense.

### Goal Idea is to reduce the annoyances and research which clang-tidy rules make sense and actually have benefits in our project. Also considering immediately resolving most if not all tidy warnings. ### Current Changes - `.clang-tidy` has changes to follow the guidelines we have been following silently. Some rules were different from how we program. - Note that naming is completely removed and won't be checked. In a future PR (where we go over the code to fix linting issues), we can reintroduce them and make sure they fit our codebase consistently. - tidy rules are not considered errors anymore (stopped compile). - CI/CD addition to build check and clang-tidy check. Tried to get clang-tidy check to show when failed, but that causes complications in its checks; so now it is simply silent when failing. - Dockerfile that creates an arch container as cache for the CI/CD, prevents redownload of dependencies each run. Instead, it runs weekly (or optionally a different interval). ### Current complications - To push the docker container build, it would require this webserver to allow larger filesize pushes (need ~2GB). Current workaround is arguably better where docker build is pushed to runner server. - Figuring out which clang-tidy rules make sense.

AramJonghu added the enhancement label 2026-06-29 00:08:05 +02:00

AramJonghu self-assigned this 2026-06-29 00:08:05 +02:00

AramJonghu added 2 commits 2026-06-29 00:08:05 +02:00

init new clang-tidy rules b943f92c0c

CI/CD for build test; includes dockerbuild to avoid docker redownloading deps. avoid-c-arrays removed 4061cc60a3

AramJonghu added 1 commit 2026-06-29 00:24:09 +02:00

ci: adjust link of dockerfile build origin d82b6da074

AramJonghu commented 2026-06-29 00:25:52 +02:00

Author

Collaborator

Copy Link

Copy Source

Current workaround is to push the Dockerfile build to my instance instead; generally better since all runners run on this server anyway.

Current workaround is to push the Dockerfile build to my instance instead; generally better since all runners run on this server anyway.

AramJonghu added 1 commit 2026-06-29 00:45:51 +02:00

adjusted CMake file to not require Cava 1ac8162c30

AramJonghu added 1 commit 2026-06-29 00:49:28 +02:00

undo of cmakelist changes, using AUR instead to resolve cava build requirement 60b0844b7f

AramJonghu added 1 commit 2026-06-29 00:59:25 +02:00

resolve clang-tidy step workflow 94100b6e2f

AramJonghu added 1 commit 2026-06-29 01:03:16 +02:00

separate workflows; build and lint clangtidy 55b73eea19

AramJonghu added 1 commit 2026-06-29 01:07:14 +02:00

in warnings will show job failed 2e2747ac04

AramJonghu added 1 commit 2026-06-29 01:15:05 +02:00

slight revert into single workflow for missing tidy check reason 7448fc101d

AramJonghu added 1 commit 2026-06-29 01:19:42 +02:00

revert; clang-tidy does on same workflow, but nonblocking afd08e8aa7

AramJonghu added 1 commit 2026-06-29 01:25:33 +02:00

clang-tidy: drop noisy checks, keep high-value only a3ca37b51a

AramJonghu added 1 commit 2026-06-29 01:30:39 +02:00

clang-tidy: disable 3 noisy bugprone checks 49010e2a43

AramJonghu commented 2026-06-29 01:39:34 +02:00

Author

Collaborator

Copy Link

Copy Source

@zach Many of the tidy rules are removed and they are not considered errors anymore. I will research further some other day (perhaps when skimming the current tidy warnings in the workflow step) to see if there is any value in the current rules. We can discuss further, but feel free to check the changes so far.

@zach Many of the tidy rules are removed and they are not considered errors anymore. I will research further some other day (perhaps when skimming the current tidy warnings in the workflow step) to see if there is any value in the current rules. We can discuss further, but feel free to check the changes so far.

AramJonghu changed title from WIP: update to clang-tidy to WIP: update to clang-tidy + workflow improvement cpp build & lint check 2026-06-29 01:51:32 +02:00

zach commented 2026-06-29 11:21:07 +02:00

Owner

Copy Link

Copy Source

This looks good so far. I don't have much of a problem with them if they're not considered errors. Is there a way to make certain diagnostics Info/Help and some Warnings? Maybe keep the diagnostics that lead to bad/buggy code as warnings, then make everything else Info/Help diagnostics.

This looks good so far. I don't have much of a problem with them if they're not considered errors. Is there a way to make certain diagnostics Info/Help and some Warnings? Maybe keep the diagnostics that lead to bad/buggy code as warnings, then make everything else Info/Help diagnostics.

AramJonghu commented 2026-06-29 11:36:20 +02:00

Author

Collaborator

Copy Link

Copy Source

Certainly possible. Previous implementation was just a wildcard (so all diagnostics were considered errors). We can pick which diagnostics we want as errors, warnings, or info/help.

Certainly possible. Previous implementation was just a wildcard (so all diagnostics were considered errors). We can pick which diagnostics we want as errors, warnings, or info/help.

AramJonghu added 1 commit 2026-06-29 19:49:03 +02:00

Merge branch 'main' into clang-tidy-fixes 25c9979674

AramJonghu added 1 commit 2026-06-29 19:56:36 +02:00

adusted workflow to fit new cmakelists 50e179e0fe

AramJonghu commented 2026-06-29 20:19:48 +02:00

Author

Collaborator

Copy Link

Copy Source

I think for consistency and readibility of PRs reasons, I should reserve fixing clang-tidy (and format) cpp/hpp files for a different PR. That would muddy this PR and add noise.

On further search; we cannot decide which become info and which become warnings. Only which ones get considered as errors. I suggest keeping as is.

I think for consistency and readibility of PRs reasons, I should reserve fixing clang-tidy (and format) cpp/hpp files for a different PR. That would muddy this PR and add noise. On further search; we cannot decide which become info and which become warnings. Only which ones get considered as errors. I suggest keeping as is.

AramJonghu marked the pull request as ready for review 2026-06-29 20:29:15 +02:00

AramJonghu requested review from zach 2026-06-29 20:29:27 +02:00

AramJonghu commented 2026-06-29 20:31:17 +02:00

Author

Collaborator

Copy Link

Copy Source

Consider a squash commit since there is a lot of noise with all the commits. A single squash commit would reduce the many commit messages with a single more informative commit message.

Consider a squash commit since there is a lot of noise with all the commits. A single squash commit would reduce the many commit messages with a single more informative commit message.

zach merged commit bc9d8af0fe into main 2026-06-29 22:10:27 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

zach

Labels

Clear labels

bug

Something isn't working

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

question

Further information is requested

wontfix

This will not be worked on

No labels enhancement

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

AramJonghu Inorishio zach

No Assignees AramJonghu

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: zach/z-bar-qt#130