Skip to content

ignore NOLINT comments with categories that come from clang-tidy#339

Merged
wjwwood merged 2 commits intomasterfrom
cpplint_ignore_clang_tidy
Mar 21, 2022
Merged

ignore NOLINT comments with categories that come from clang-tidy#339
wjwwood merged 2 commits intomasterfrom
cpplint_ignore_clang_tidy

Conversation

@wjwwood
Copy link
Copy Markdown
Contributor

@wjwwood wjwwood commented Nov 11, 2021

So, I'm still running into issues with clang-tidy and cpplint colliding when using NOLINT comments. This is an idea on how to handle it and I've made an upstream issue to see what they think as well: cpplint/cpplint#184

@nuclearsandwich @mjeronimo FYI

Signed-off-by: William Woodall <william@osrfoundation.org>
@nuclearsandwich nuclearsandwich self-requested a review November 11, 2021 18:12
@mjeronimo
Copy link
Copy Markdown

Looks like a good idea to me. Also like the idea of pushing it upstream to the tool as well.

@wjwwood wjwwood marked this pull request as ready for review March 16, 2022 17:38
@wjwwood
Copy link
Copy Markdown
Contributor Author

wjwwood commented Mar 16, 2022

Moving this into review, I would appreciate feedback.

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@wjwwood wjwwood requested a review from mjeronimo March 16, 2022 17:41
Signed-off-by: William Woodall <william@osrfoundation.org>
Copy link
Copy Markdown
Contributor

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

No objections to the implementation. It'll be great if this becomes part of our upstream fork.

@mjeronimo
Copy link
Copy Markdown

Looks good to me as well.

@wjwwood
Copy link
Copy Markdown
Contributor Author

wjwwood commented Mar 21, 2022

The test failures look unrelated to me and were not present on Windows. Thanks for the reviews guys. I'll see about following up on our fork if this ends up in a future version of upstream, which seems likely.

@wjwwood wjwwood merged commit 467a5d5 into master Mar 21, 2022
@delete-merged-branch delete-merged-branch bot deleted the cpplint_ignore_clang_tidy branch March 21, 2022 16:22
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