-
Notifications
You must be signed in to change notification settings - Fork 212
refactor: Add clang-tidy checks #195
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
Conversation
sea-bass
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting this is introducing ABI incompatibilities, presumably even to older "stable" distros. Are we okay with this? I personally think this is fine unless there are other packages released to the buildfarm that rely on this. But if this is a true "leaf" package, then let's send it.
There are no packages in rosdistro that link against this library. Also, The API and ABI were already broken by the latest refactor (#192) |
sea-bass
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR adds clang-tidy check using
ament_clang_tidy. The check is disabled by default and can be enabled usingENABLE_CLANG_TIDYCMake option. Two reasons behind it:The motivation behind clang-tidy was the readability-identifier-naming check which codifies naming conventions as requested in this discussion
The few other basic checks like
google-*,misc-*,readability-*seem to completely supersedecpplintso I removed it.