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

V2 TO V3 fragment changes for extension and server directory under test/.. #12058

Conversation

ankatare
Copy link
Contributor

@ankatare ankatare commented Jul 13, 2020

Signed-off-by: Abhay Narayan Katare [email protected]

Commit Message: v2 to v3 fragment changes for extension and server directory test cases.
Additional Description:
Risk Level: Low
Testing: unit and format
Docs Changes:NA
Release Notes: NA
[Optional Runtime guard:]
Optional Fixes #10843
[Optional Deprecated:]

@junr03 junr03 self-assigned this Jul 13, 2020
@ankatare ankatare force-pushed the v2_to_v3_fragment_changes_extensions_directory branch from e9f94ea to 937e11d Compare July 14, 2020 07:21
@ankatare
Copy link
Contributor Author

@junr03 seems some env issues in this PR.
https://dev.azure.com/cncf/envoy/_build/results?buildId=44878&view=results

please suggest if it is good to go with code review.

@junr03
Copy link
Member

junr03 commented Jul 18, 2020

Try merging master again?

@ankatare ankatare force-pushed the v2_to_v3_fragment_changes_extensions_directory branch from 937e11d to 7a61a59 Compare July 18, 2020 08:37
@ankatare
Copy link
Contributor Author

@junr03 ok.

@ankatare ankatare force-pushed the v2_to_v3_fragment_changes_extensions_directory branch from 7a61a59 to 588b1aa Compare July 19, 2020 08:16
Signed-off-by: Abhay Narayan Katare <[email protected]>
@ankatare
Copy link
Contributor Author

@junr03 merged from master. again seems some different env issues. please suggest if it is ok to review code.

@junr03
Copy link
Member

junr03 commented Jul 20, 2020

@ankatare looks like a valid clang-tidy failure. Not because of your change. The file was already invalid, but changing it made it so that clang-tidy ran over it and detected already existing failures. Do you mind fixing?

@ankatare
Copy link
Contributor Author

@junr03 sure, please suggest how to fix it , any pointers would be helpful.

@ankatare
Copy link
Contributor Author

@junr03 i tried below command to fix issue
clang-tidy -fix-errors -checks='readability-identifier-naming' test/extensions/filters/network/thrift_proxy/filters/ratelimit/ratelimit_test.cc.
That fixes clang-tidy error. but while unit testing it with bazel test it is failing locally.
would you please suggest what to do as in this case as i can't push test file now.

@junr03
Copy link
Member

junr03 commented Jul 23, 2020

@ankatare what is the failure trace?

@ankatare ankatare force-pushed the v2_to_v3_fragment_changes_extensions_directory branch 4 times, most recently from fad74c0 to 951c1df Compare July 24, 2020 06:39
Signed-off-by: Abhay Narayan Katare <[email protected]>
@ankatare ankatare force-pushed the v2_to_v3_fragment_changes_extensions_directory branch from 951c1df to 6c96003 Compare July 24, 2020 11:33
@ankatare
Copy link
Contributor Author

@junr03 Build completed with master in sync and resolved clang-tidy issues. please review.

@junr03 junr03 merged commit ae0c311 into envoyproxy:master Jul 31, 2020
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
…st/.. (envoyproxy#12058)

Commit Message: v2 to v3 fragment changes for extension and server directory test cases.
Risk Level: Low
Testing: unit and format
Fixes envoyproxy#10843

Signed-off-by: Abhay Narayan Katare <[email protected]>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
…st/.. (envoyproxy#12058)

Commit Message: v2 to v3 fragment changes for extension and server directory test cases.
Risk Level: Low
Testing: unit and format
Fixes envoyproxy#10843

Signed-off-by: Abhay Narayan Katare <[email protected]>
Signed-off-by: chaoqinli <[email protected]>
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.

Convert v2 API test fragments to v3
2 participants