Skip to content

Conversation

@ReginaldWang
Copy link
Contributor

@ReginaldWang ReginaldWang commented Jan 7, 2026

Clang-Tidy Linter

Problem and Scope

Meant to solve issue #163. Clang-tidy system is set up.

Description

Default clang-tidy file is added. Should cover most limitations of CodeQL. Github action is added (pretty simple, runs on pull request).

Gotchas and Limitations

I have no idea how it interacts with the other .yml files.

Testing

  • HOOTL testing
  • HITL testing
  • Human tested

Testing Details

Larger Impact

Additional Context and Ticket

@ReginaldWang ReginaldWang marked this pull request as draft January 7, 2026 18:11
@dchansen06 dchansen06 linked an issue Jan 7, 2026 that may be closed by this pull request
@dchansen06 dchansen06 added this to the Monorepo Niceties milestone Jan 7, 2026
@dchansen06 dchansen06 added Enhancement New feature or request GitHub Meta, anything related to or dealing with GitHub HOOTL Testing Having to do with or interacting with HOOTL testing Big Fry Something that is complex and/or large Pipe Dream Would be amazing... but realistically... it might be dubious to get it on the car 4 REDUCED Explicitly not a priority but would be nice to do down the road labels Jan 7, 2026
@dchansen06 dchansen06 added 3 NORMAL Important but not really a priority and removed 4 REDUCED Explicitly not a priority but would be nice to do down the road labels Jan 7, 2026
@dchansen06
Copy link
Contributor

Updated your branch with a rebase to keep it updated in prep for #165

@dchansen06 dchansen06 changed the title Add .clang-tidy configuration file Add Clang-Tidy GitHub Action Linter Jan 9, 2026
- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install -y clang-tidy cmake ninja-build
Copy link
Contributor

Choose a reason for hiding this comment

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

These 3 pieces of software are installed by default on the ubuntu-latest runner you do not need to install them again see the docs for a list of other installed packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

I imagine as part of this you will find actual bugs in our codebase, when you do please document them to the best of your ability in a new GitHub issue for each, thanks!

@dchansen06
Copy link
Contributor

Updating with rebase again

@dchansen06
Copy link
Contributor

Updated with rebase

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Added steps to install dependencies and configure the build.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Replaced clang-tidy-review actions with a direct clang-tidy command.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Removed installation of clang-tidy and cmake from dependencies.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Removed the 'Configure' step from the Linter workflow.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Removed AnalyzeTemporaryDtors option from clang-tidy configuration.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
Added a configuration step and improved clang-tidy invocation.

Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
dchansen06 and others added 2 commits January 19, 2026 20:06
…in permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: ReginaldWang <114448545+ReginaldWang@users.noreply.github.com>
@ReginaldWang ReginaldWang marked this pull request as ready for review January 23, 2026 05:01
@dchansen06
Copy link
Contributor

I noticed your PR runs only on changed files (which seems reasonable) so I ran it over all of them and got... not good results clang-tidy-log.txt namely seeing error: too many errors emitted, stopping now [clang-diagnostic-error] seems pretty bad

What options are there? Is there a way to hide less important errors? I already filtered out Lib/Vendor code when I ran that test...

Copy link
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

May need to update the .clang-tidy or something else, I am not sure, but there are a lot of errors and warnings from the linter which is not good

To see them on the GitHub Action run

git ls-files '*.c' '*.h' | grep -v "Lib/Vendor" | xargs clang-tidy -p build/Debug

contents: read

jobs:
build:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
build:
tidy:

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please add a name like the other ones name: Clang-Tidy


- name: Run clang-tidy
run: |
files=$(git diff --name-only origin/${{ github.base_ref }} | grep -E '\.(cpp|cc|cxx|c)$' || true)
Copy link
Contributor

Choose a reason for hiding this comment

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

For getting changed files please use an action like changed-files it is a bit cleaner

Can you for now please make it

  • Auto fix what it can (and save changes)
  • Fail the job on failure

echo "No C/C++ source files changed"
exit 0
fi
clang-tidy -p build $files
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
clang-tidy -p build $files
clang-tidy -p build/Debug $files

release: '12.2.Rel1'

- name: Configure
run: cmake --preset Debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to avoid linting on just one preset?

@dchansen06 dchansen06 marked this pull request as draft January 23, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 NORMAL Important but not really a priority Big Fry Something that is complex and/or large Enhancement New feature or request GitHub Meta, anything related to or dealing with GitHub HOOTL Testing Having to do with or interacting with HOOTL testing Pipe Dream Would be amazing... but realistically... it might be dubious to get it on the car

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setup Clang-Tidy System and Incorporate A GitHub Action

2 participants