Skip to content

Added initial .clang-tidy file - #2595

Merged
Alexays merged 1 commit into
Alexays:masterfrom
zjeffer:master
Dec 4, 2023
Merged

Alexays merged 1 commit into
Alexays:masterfrom
zjeffer:master

Conversation

@zjeffer

@zjeffer zjeffer commented Oct 21, 2023

Copy link
Copy Markdown
Contributor

More info here: #2591

@khaneliman

Copy link
Copy Markdown
Contributor

Looks like it's already working in the linter :P

@zjeffer

zjeffer commented Oct 22, 2023

Copy link
Copy Markdown
Contributor Author

Looks like it's already working in the linter :P

No, that's clang-format detecting a file in the current master not being formatted correctly. I had the same linter error in my other PR.

If we would add a GitHub action that checks clang-tidy, we'd see lots more errors :D

@Alexays

Alexays commented Oct 23, 2023

Copy link
Copy Markdown
Owner

I'm ok with making a big PR that fixes all the lint at once as well as adding other rules to improve code quality.

@Alexays

Alexays commented Oct 23, 2023

Copy link
Copy Markdown
Owner

And updating Github Action to reflect that.

@zjeffer

zjeffer commented Nov 2, 2023

Copy link
Copy Markdown
Contributor Author

@Alexays I added the casing hints to the file, to show warnings in the IDE when the casing of the name of a class, function, struct, member variable, ... is inconsistent with the clang-tidy file. I based my choices on the existing code, to keep the impact to the code the lowest.

One thing I'm unsure of: I added a m_ as a PrivateMemberPrefix, so member variables have to start with m_, which I think makes the code much clearer. I don't see it used very much in this codebase, though, so let me know if you think this should be included.

Can you also check whether the other CheckOptions make sense to you? Let me know if you want to make any changes.

I'll start tidying-up the code (in a separate commit) once you're sure about all the options.

@Alexays
Alexays merged commit 4846ff7 into Alexays:master Dec 4, 2023
@Alexays

Alexays commented Dec 4, 2023

Copy link
Copy Markdown
Owner

LGTM, we'll make small changes to the config if we realise it's too much.

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.

3 participants