Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a clang-tidy config file #1434

Closed
wants to merge 1 commit into from
Closed

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Jan 9, 2024

All checks are enabled except for those that cause any warning.

This is a starting point, some of the currently-supressed warnings can be fixed hereafter.

@mcuee mcuee added the Misc Whitespace, typo, coding style, etc label Jan 10, 2024
@Youw
Copy link
Member

Youw commented Jan 10, 2024

I think it is best to introduce things like this along with the CI script to run/check it.
It is not implicitly used by any of the existing tools, right?

@Youw
Copy link
Member

Youw commented Jan 10, 2024

And I agree - a very good starting point.

@seanm
Copy link
Contributor Author

seanm commented Jan 10, 2024

I think it is best to introduce things like this along with the CI script to run/check it.

Agreed. But I don't know anything about github CI, hopefully someone else can integrate this.

At least this file allows developers to use clang-tidy locally.

It is not implicitly used by any of the existing tools, right?

Correct; as far as I know.

All checks are enabled except for those that cause any warning.

This is a starting point, some of the currently-supressed warnings can be fixed hereafter.
@hjelmn
Copy link
Member

hjelmn commented Jan 20, 2024

Do we want to take this one step further and add clang-format as a prerequisite?

@seanm
Copy link
Contributor Author

seanm commented Jan 20, 2024

Do we want to take this one step further and add clang-format as a prerequisite?

Did you mean clang-tidy? clang-format is an orthogonal discussion. I had started work on clang-format, but waiting reply here: #1421 (comment)

@hjelmn
Copy link
Member

hjelmn commented Jan 22, 2024

I mean both. I kind of want to set minimum standards on commits.

@seanm
Copy link
Contributor Author

seanm commented Jan 22, 2024

I mean both. I kind of want to set minimum standards on commits.

I'm all for clang-format too. But it's unrelated to clang-tidy and this PR. Let's move the discussion here: #1442

@seanm
Copy link
Contributor Author

seanm commented Feb 2, 2024

@tormodvolden for example, if this was merged to master, then I (and others) could be using clang-tidy to check our work in other branches.

Copy link
Member

@Youw Youw left a comment

Choose a reason for hiding this comment

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

@seanm
Copy link
Contributor Author

seanm commented Feb 24, 2024

@tormodvolden @hjelmn is there any reason to not just merge this? It can't break anything else, it's just a config file...

@tormodvolden
Copy link
Contributor

Can you please give some quick instructions how to use it? I installed clang-tidy (and half a gigabyte of dependencies) and naively tried "clang-tidy" or "clang-tidy <list of .c and .h files>" but I got a load of warnings, and some hinting about a database.

@seanm
Copy link
Contributor Author

seanm commented Feb 27, 2024

Can you please give some quick instructions how to use it? I installed clang-tidy (and half a gigabyte of dependencies) and naively tried "clang-tidy" or "clang-tidy <list of .c and .h files>" but I got a load of warnings, and some hinting about a database.

It's referring to a 'compilation database'.

I used the cmake fork of libusb to generate one.

Thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Misc Whitespace, typo, coding style, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants