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

Check for all things with cppcheck #2833

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

tobbi
Copy link
Member

@tobbi tobbi commented Mar 17, 2024

No description provided.

@mrkubax10 mrkubax10 added type:patch status:needs-review Work needs to be reviewed by other people category:tests labels Mar 17, 2024
@tobbi
Copy link
Member Author

tobbi commented Mar 17, 2024

I don't know how to add the src directory as an include base for the cppcheck thing.

[ci skip]
@tobbi
Copy link
Member Author

tobbi commented Mar 18, 2024

Well, this works, but...

@tobbi
Copy link
Member Author

tobbi commented Mar 18, 2024

I don't know if we want all the reported errors in there.

@tobbi
Copy link
Member Author

tobbi commented Mar 18, 2024

@tobbi
Copy link
Member Author

tobbi commented Mar 18, 2024

I feel like this adds way too many warnings. Maybe we should cut down the number of things it checks?

@HybridDog
Copy link
Contributor

HybridDog commented Apr 13, 2024

I don't know how to add the src directory as an include base for the cppcheck thing.

It is possible to run cmake with -DCMAKE_EXPORT_COMPILE_COMMANDS=1 and pass the compile commands with --project=compile_commands.json to cppcheck. I don't know if using cppcheck this way is still fast.

I feel like this adds way too many warnings. Maybe we should cut down the number of things it checks?

I think it is important that the cppcheck linting passes in the master branch and can only fail in a Pull Request, telling the developer that further changes are needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tests status:needs-review Work needs to be reviewed by other people type:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants