Skip to content

remove google style from clang-tidy default settings#337

Merged
wjwwood merged 1 commit intomasterfrom
remove_google_style_from_default_settings
Nov 3, 2021
Merged

remove google style from clang-tidy default settings#337
wjwwood merged 1 commit intomasterfrom
remove_google_style_from_default_settings

Conversation

@wjwwood
Copy link
Copy Markdown
Contributor

@wjwwood wjwwood commented Nov 1, 2021

This also removes the need for default config file.

I propose this change because:

  • cpplint.py covers these Google style-like checks (things like using explicit on a constructor with one argument)
  • these are often false positives, and need to be suppressed, but the suppression mechanism (// NOLINT(rule)) is problematic because the name of the rules are different for cpplint.py and clang-tidy's version of the rules
  • when providing multiple rules clang-tidy emits warnings that it doesn't know the rule name used by cpplint.py, causing more unhelpful warnings

The simplest solution, in my opinion, is to remove this from the default settings, rely on cpplint.py for these kinds of checks, and let individual users add the Google style checks to their on clang-tidy config files, if they wish to use them.

@nuclearsandwich FYI

…or default config file

Signed-off-by: William Woodall <william@osrfoundation.org>
@wjwwood wjwwood self-assigned this Nov 1, 2021
Copy link
Copy Markdown
Contributor

@audrow audrow left a comment

Choose a reason for hiding this comment

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

This looks good to me.

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.

Thanks @wjwwood!

@wjwwood
Copy link
Copy Markdown
Contributor Author

wjwwood commented Nov 3, 2021

CI:

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

@wjwwood wjwwood merged commit 1525d38 into master Nov 3, 2021
@delete-merged-branch delete-merged-branch bot deleted the remove_google_style_from_default_settings branch November 3, 2021 05:26
@nyxrobotics
Copy link
Copy Markdown

Hello, nice to meet you. I have one question: Based on your explanation, it seems like configuring clang-tidy would be the better approach. Could you clarify why you prefer to use cpplint instead? Is this part of a milestone where you plan to eventually transition to clang-tidy in the future?

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.

4 participants