Skip to content

Added httpConnectionManagerConfigHelper to refactor config_test#11161

Merged
mattklein123 merged 3 commits into
envoyproxy:masterfrom
mk46:config_test_refactoring
Jun 12, 2020
Merged

Added httpConnectionManagerConfigHelper to refactor config_test#11161
mattklein123 merged 3 commits into
envoyproxy:masterfrom
mk46:config_test_refactoring

Conversation

@mk46
Copy link
Copy Markdown
Contributor

@mk46 mk46 commented May 12, 2020 •
edited
Loading

Signed-off-by: Manish Kumar manish.kumar1@india.nec.com

Commit Message: Refactored config_test.cc
Additional Description:
created createHttpConnectionManagerConfig() to improve readability.

Risk Level:
Testing: unit test
Docs Changes:
Release Notes:
includes partial fix to #11109

Added httpConnectionManagerConfigHelper to refactor config_test
8dbe0ea 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@dio
Copy link
Copy Markdown
Member

dio commented May 12, 2020

@mk46, thanks! Could you update the PR description? Thanks!

dio
dio reviewed May 12, 2020
View reviewed changes
Comment thread test/extensions/filters/network/http_connection_manager/config_test.cc Outdated
@dio dio self-assigned this May 12, 2020
@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented May 13, 2020 •
edited
Loading

Hi @dio, I found
HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_, date_provider_, route_config_provider_manager_, scoped_routes_config_provider_manager_, http_tracer_manager_);
is 48 times. Could you suggest how to refactor it? Thanks!

dio
dio reviewed May 13, 2020
View reviewed changes
Comment thread test/extensions/filters/network/http_connection_manager/config_test.cc Outdated
Changed httpConnectionManagerConfigHelper() to createHttpConnectionMa…
829a3e3 
…nagerConfig()

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@dio
Copy link
Copy Markdown
Member

dio commented May 15, 2020

@mk46 sorry, the Commit Message in the description, please? As per: https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md#commit-message

@mk46
Copy link
Copy Markdown
Contributor Author

mk46 commented May 20, 2020

@dio PTAL

@mk46 mk46 requested a review from dio May 29, 2020 12:43
@stale
Copy link
Copy Markdown

stale Bot commented Jun 5, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale Bot added the stale stalebot believes this issue/PR has not been touched recently label Jun 5, 2020
dio
dio approved these changes Jun 10, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks!

@stale stale Bot removed the stale stalebot believes this issue/PR has not been touched recently label Jun 10, 2020
@dio
Copy link
Copy Markdown
Member

dio commented Jun 10, 2020

Sorry, @mk46 could you merge master, please? Thanks!

Merge remote-tracking branch master into config_test_refactoring
67ffd01 
Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
@mk46 mk46 requested a review from dio June 11, 2020 07:51
@dio
Copy link
Copy Markdown
Member

dio commented Jun 11, 2020

/azp run envoy-presubmit

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Jun 11, 2020

Azure Pipelines successfully started running 1 pipeline(s).

@mattklein123 mattklein123 merged commit 47463bc into envoyproxy:master Jun 12, 2020
arthuryan-k pushed a commit to arthuryan-k/envoy that referenced this pull request Jun 15, 2020
@arthuryan-k
Added httpConnectionManagerConfigHelper to refactor config_test (envo…
5bbf892 
…yproxy#11161)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Signed-off-by: Arthur Yan <arthuryan@google.com>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jun 24, 2020
@yashwant121
Added httpConnectionManagerConfigHelper to refactor config_test (envo…
f571310 
…yproxy#11161)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
songhu pushed a commit to songhu/envoy that referenced this pull request Jun 25, 2020
@songhu
Added httpConnectionManagerConfigHelper to refactor config_test (envo…
5200d74 
…yproxy#11161)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jul 24, 2020
@yashwant121
Added httpConnectionManagerConfigHelper to refactor config_test (envo…
37a348b 
…yproxy#11161)

Signed-off-by: Manish Kumar <manish.kumar1@india.nec.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alyssawilk alyssawilk Awaiting requested review from alyssawilk

@mattklein123 mattklein123 Awaiting requested review from mattklein123

1 more reviewer

@dio dio dio approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dio dio

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mk46 @dio @mattklein123