Skip to content

[Clang-Tidy] Readability Improvements#1220

Merged
Skylion007 merged 3 commits intomasterfrom
clang_tidy_readability_checks
May 7, 2021
Merged

[Clang-Tidy] Readability Improvements#1220
Skylion007 merged 3 commits intomasterfrom
clang_tidy_readability_checks

Conversation

@Skylion007
Copy link
Copy Markdown
Contributor

@Skylion007 Skylion007 commented May 7, 2021

Motivation and Context

  • Add readability checks to Clang-Tidy. Starting with some conservative ones like making sure all auto are properly qualified (ie shows if they are const or a reference etc...).
  • No redundant_cstr from strings
  • Make sure all const methods are marked as such. Seems to be a very common reviewer suggestion.

How Has This Been Tested

  • Clang-Tidy and CI

Types of changes

  • Docs change / refactoring / dependency upgrade

Checklist

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 7, 2021
@Skylion007 Skylion007 requested a review from bigbike May 7, 2021 17:28
Copy link
Copy Markdown
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

LGTM. I wasn't aware of this best practice for auto usage, but it makes sense.

Copy link
Copy Markdown
Contributor

@mosra mosra left a comment

Choose a reason for hiding this comment

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

Good stuff. 👍

I'd personally like to avoid auto in some places (i.e., where the type is not immediately clear from the few lines of diff context), but that's unrelated to this change. This is great.

Copy link
Copy Markdown
Contributor

@jturner65 jturner65 left a comment

Choose a reason for hiding this comment

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

LGTM

@Skylion007 Skylion007 merged commit 5cb101a into master May 7, 2021
@Skylion007 Skylion007 deleted the clang_tidy_readability_checks branch May 7, 2021 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants