Skip to content

clang-tidy: check casing on class names#8411

Merged
lizan merged 3 commits intoenvoyproxy:masterfrom
derekargueta:dereka/clang-tidy-identifier-class-name
Oct 4, 2019
Merged

clang-tidy: check casing on class names#8411
lizan merged 3 commits intoenvoyproxy:masterfrom
derekargueta:dereka/clang-tidy-identifier-class-name

Conversation

@derekargueta
Copy link
Copy Markdown
Member

@derekargueta derekargueta commented Sep 27, 2019

Description: verify CamelCase for class names.
Risk Level: low
Testing: existing
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Derek Argueta dereka@pinterest.com

@derekargueta
clang-tidy: check casing on class names
db93632 
Signed-off-by: Derek Argueta <dereka@pinterest.com>
lizan
lizan reviewed Sep 27, 2019
View reviewed changes
test/common/common/log_macros_test.cc Outdated
};

logTestClass testObj;
LogTestClass testObj;
Copy link
Copy Markdown
Member

@lizan lizan Sep 27, 2019

Choose a reason for hiding this comment

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

s/testObj/test_obj/ I think you'll catch this in next identifier-naming PR but feel free to fix here too.

@derekargueta
Copy link
Copy Markdown
Member Author

derekargueta commented Oct 1, 2019 •
edited
Loading

I might need to revisit this check another time - clang-tidying source/common/common/fmt.h in isolation has an issue with #include "absl/strings/string_view.h" because the bazel target is an envoy_basic_cc_library, a thin wrapper around native.cc_library which doesn't know of abseil. Trying to move it to an envoy_cc_library causes a circular dependency ¯\_(ツ)_/¯

This isn't an effect of clang-tidy, just an issue in the build system that seems to have relied on transitive dependencies for getting abseil.

@lizan
Copy link
Copy Markdown
Member

lizan commented Oct 1, 2019

@derekargueta I think you may add abseil_strings deps to envoy_basic_cc_library, that shouldn't introduce circular dependency.

@derekargueta
set abseil_strings and fmtlib as dependencies of fmtlib
32e48a9 
Signed-off-by: Derek Argueta <dereka@pinterest.com>
@derekargueta
fix variable casing
86677af 
Signed-off-by: Derek Argueta <dereka@pinterest.com>
@lizan lizan merged commit 0360afe into envoyproxy:master Oct 4, 2019
@derekargueta derekargueta deleted the dereka/clang-tidy-identifier-class-name branch October 5, 2019 07:55
nandu-vinodan pushed a commit to nandu-vinodan/envoy that referenced this pull request Oct 17, 2019
@derekargueta @nandu-vinodan
clang-tidy: check casing on class names (envoyproxy#8411)
ce362ad 
Description: verify `CamelCase` for class names.
Risk Level: low
Testing: existing
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Derek Argueta <dereka@pinterest.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lizan lizan lizan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@lizan lizan

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@derekargueta @lizan