Skip to content

Change HdsCluster::cluster_ to a value instead of reference#12137

Merged
snowp merged 1 commit intoenvoyproxy:masterfrom
xdzhai:master
Jul 20, 2020
Merged

Change HdsCluster::cluster_ to a value instead of reference#12137
snowp merged 1 commit intoenvoyproxy:masterfrom
xdzhai:master

Conversation

@xdzhai
Copy link
Contributor

@xdzhai xdzhai commented Jul 16, 2020

Commit Message:
Change HdsCluster to store Cluster config proto with unique_ptr

Additional Description:
Currently it's created locally and assigned to a const reference, may be better to use unique_ptr .

Risk Level: Low
Testing: Passed hds_test and hds_integration_test.
Docs Changes: N/A
Release Notes: N/A

@xdzhai
Copy link
Contributor Author

xdzhai commented Jul 17, 2020

/retest

@repokitteh-read-only
Copy link

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #12137 (comment) was created by @xdzhai.

see: more, trace.

Copy link
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

Thanks, just one question on the approach

Signed-off-by: Xiaodong Zhai <xdzhai@google.com>
@xdzhai xdzhai changed the title Change HdsCluster to store Cluster config proto with unique_ptr Change HdsCluster::cluster_ to a value instead of reference Jul 18, 2020
@xdzhai xdzhai requested a review from snowp July 18, 2020 00:22
Copy link
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

Thanks!

@snowp snowp merged commit 3e92500 into envoyproxy:master Jul 20, 2020
KBaichoo pushed a commit to KBaichoo/envoy that referenced this pull request Jul 30, 2020
…oyproxy#12137)

Signed-off-by: Xiaodong Zhai <xdzhai@google.com>
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
scheler pushed a commit to scheler/envoy that referenced this pull request Aug 4, 2020
…oyproxy#12137)

Signed-off-by: Xiaodong Zhai <xdzhai@google.com>
Signed-off-by: scheler <santosh.cheler@appdynamics.com>
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.

2 participants