Skip to content

Conversation

@bjsowa
Copy link
Member

@bjsowa bjsowa commented Jan 6, 2026

This PR adds clang-tidy check using ament_clang_tidy. The check is disabled by default and can be enabled using ENABLE_CLANG_TIDY CMake option. Two reasons behind it:

  • The check is quite slow (~100 seconds in CI)
  • In CI, we enable the check only for Rolling distribution to avoid having to deal with multiple different versions of clang-tidy

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 supersede cpplint so I removed it.

@bjsowa bjsowa marked this pull request as ready for review January 7, 2026 09:15
@bjsowa bjsowa requested review from EzraBrooks and sea-bass January 7, 2026 09:15
sea-bass
sea-bass previously approved these changes Jan 10, 2026
Copy link
Contributor

@sea-bass sea-bass left a 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.

@bjsowa
Copy link
Member Author

bjsowa commented Jan 10, 2026

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)

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

LGTM!

@bjsowa bjsowa merged commit eef3dc2 into ros2 Jan 12, 2026
16 checks passed
@bjsowa bjsowa deleted the refactor/add-clang-tidy branch January 12, 2026 02:45
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