Skip to content

Conversation

dmah42
Copy link
Member

@dmah42 dmah42 commented Feb 10, 2025

No description provided.

@@ -35,4 +35,4 @@ jobs:
- name: run
shell: bash
working-directory: ${{ runner.workspace }}/_build
run: run-clang-tidy -checks=*,-clang-analyzer-deadcode*,-clang-analyzer-optin*
run: run-clang-tidy -checks=*,-clang-analyzer-deadcode*,-clang-analyzer-optin*,-fuchsia-*,-llvmlibc-*,-modernize-use-trailing-return-type
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might want to place this in a .clang-tidy file in the root source directory

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah i'm also going to reduce the number of checks we're running.

@LebedevRI
Copy link
Collaborator

Just a general comment: it might be best to approach this as one check per PR,
otherwise it may turn into too-large-to-meaningfully review.

@dmah42
Copy link
Member Author

dmah42 commented Feb 10, 2025

Just a general comment: it might be best to approach this as one check per PR, otherwise it may turn into too-large-to-meaningfully review.

yeah, it's a draft PR so i can split it up later.

@@ -35,4 +35,4 @@ jobs:
- name: run
shell: bash
working-directory: ${{ runner.workspace }}/_build
run: run-clang-tidy -checks=*,-clang-analyzer-deadcode*,-clang-analyzer-optin*
run: run-clang-tidy -config-file=${{ runner.workspace }}/.clang-tidy
Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be picked up automatically

Copy link
Member Author

Choose a reason for hiding this comment

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

it's not finding it, which suggests the same as when i passed it explicitly: that it's not running with the root as a parent directory, somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

it finds it locally, fwiw

LebedevRI
LebedevRI previously approved these changes Feb 11, 2025
Copy link
Collaborator

@LebedevRI LebedevRI left a comment

Choose a reason for hiding this comment

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

Awesome!

@dmah42
Copy link
Member Author

dmah42 commented Feb 11, 2025

ok fixed it. clang-tidy is now running properly with the common config.

@dmah42 dmah42 changed the title clang-tidy run clang-tidy using a common config and reduced set of tests Feb 11, 2025
@dmah42 dmah42 marked this pull request as ready for review February 11, 2025 00:28
@dmah42 dmah42 merged commit a125fb6 into main Feb 11, 2025
96 of 101 checks passed
@dmah42 dmah42 deleted the enum_narrowing branch February 11, 2025 00:32
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