Skip to content

[Lint] run clang-tidy in scripts/format.h, update clang-tidy rules#19055

Merged
simon-mo merged 5 commits intoray-project:masterfrom
mwtian:clang-tidy
Oct 5, 2021
Merged

[Lint] run clang-tidy in scripts/format.h, update clang-tidy rules#19055
simon-mo merged 5 commits intoray-project:masterfrom
mwtian:clang-tidy

Conversation

@mwtian
Copy link
Copy Markdown
Member

@mwtian mwtian commented Oct 2, 2021 •
edited
Loading

Why are these changes needed?

  • Make it more convenient to run clang-tidy by including it in scripts/format.h.
  • Use the same script to run clang-format in scripts/format.h as in CI. Currently they use different logic to run clang-format on changes, which can lead to different results.
  • Include additional clang-tidy rules to catch more readability issues.
  • Improve messages emitted during lint, to clarify installation instructions for missing tools, and make lint errors stand out more.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@mwtian mwtian added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 2, 2021
@mwtian mwtian removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 4, 2021
@mwtian mwtian mentioned this pull request Oct 4, 2021
Merged
6 tasks
@mwtian mwtian added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed tests-ok The tagger certifies test failures are unrelated and assumes personal liability. labels Oct 4, 2021
scv119
scv119 approved these changes Oct 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@scv119 scv119 left a comment

Choose a reason for hiding this comment

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

@mwtian thanks a lot for keep improving this!

@simon-mo simon-mo merged commit 5d9e3a0 into ray-project:master Oct 5, 2021
@mwtian mwtian deleted the clang-tidy branch October 5, 2021 22:08
ericl added a commit to ericl/ray that referenced this pull request Oct 5, 2021
@ericl
Revert "[Lint] run clang-tidy in scripts/format.h, update clang-t…
52e1a26 
…idy rules (ray-project#19055)"

This reverts commit 5d9e3a0.
@ericl ericl mentioned this pull request Oct 5, 2021
Merged
ericl added a commit that referenced this pull request Oct 5, 2021
@ericl
Revert "[Lint] run clang-tidy in scripts/format.h, update clang-t…
f8a91c7 
…idy rules (#19055)" (#19119)

This reverts commit 5d9e3a0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scv119 scv119 scv119 approved these changes

@rkooo567 rkooo567 rkooo567 approved these changes

@simon-mo simon-mo simon-mo approved these changes

Assignees

@scv119 scv119

@rkooo567 rkooo567

@simon-mo simon-mo

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mwtian @scv119 @rkooo567 @simon-mo