Skip to content
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

Fix clang-tidy #7475

Merged
merged 4 commits into from
Jul 8, 2019
Merged

Conversation

derekargueta
Copy link
Member

Description: #7442 introduced a regression by breaking the syntax in .clang-tidy. For the period of about 3 days, clang-tidy only ran warnings for clang-analyzer-*. In one of my recent PRs I noticed the output on CI containing Invalid Argument. This PR fixes clang-tidy, and I've done a full run of master to ensure there weren't any regressions.
Risk Level: Low
Testing: Ran clang-tidy locally
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
Signed-off-by: Derek Argueta <dereka@pinterest.com>
@mattklein123
Copy link
Member

Thanks for catching this and fixing, can you make it so the clang-tidy job fails if there is a syntax error?

/wait

@derekargueta
Copy link
Member Author

Yeah, I was looking for a strict-type option on clang-tidy to fail if the given config is bad, but doesn't look like there's one. Next best thing I can do is a quick YAML syntax check in the CI scripts prior to running clang-tidy.

Signed-off-by: Derek Argueta <dereka@pinterest.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit f683743 into envoyproxy:master Jul 8, 2019
@derekargueta derekargueta deleted the dereka/fix-clang-tidy branch November 26, 2019 09:14
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.

3 participants