-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang-tidy: modernize-loop-convert #7790
clang-tidy: modernize-loop-convert #7790
Conversation
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
/retest |
🔨 rebuilding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with small typo, thanks!
/wait
@@ -23,11 +23,11 @@ bool RoleBasedAccessControlEngineImpl::allowed(const Network::Connection& connec | |||
std::string* effective_policy_id) const { | |||
bool matched = false; | |||
|
|||
for (auto it = policies_.begin(); it != policies_.end(); it++) { | |||
if (it->second.matches(connection, headers, metadata)) { | |||
for (const auto& policie : policies_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a/policie/policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wonder why spelling CI didn't catch this when it caught another one that was very similar: 5203b0e
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Description: Enable and fix existing cases of
modernize-loop-convert
. This clang-tidy check looks for raw for-loops that can be replaced with range-based loops.https://clang.llvm.org/extra/clang-tidy/checks/modernize-loop-convert.html
There are a few configuration options for this check (see above link for more details) but I think the defaults are perfectly suitable.
Risk Level: Low
Testing: Existing
Docs Changes: N/A
Release Notes: N/A
[Optional Fixes #Issue]
[Optional Deprecated:]
Signed-off-by: Derek Argueta dereka@pinterest.com