Skip to content

Conversation

mum4k
Copy link
Collaborator

@mum4k mum4k commented Jan 12, 2021

  • Envoy::StreamInfo::StreamInfoImpl no longer contains methods that set local and remote socket addresses. It instead requires a Envoy::Network::SocketAddressProviderSharedPtr with the addresses already set. Pass in a Envoy::Network::SocketAddressSetterImpl so that we can continue setting the remote address for tests.
  • Copied .bazelversion and .bazelrc from Envoy.

Signed-off-by: Jakub Sobon mumak@google.com

- Envoy::StreamInfo::StreamInfoImpl no longer contains methods that set
local and remote socket addresses. It instead requires a
Envoy::Network::SocketAddressProviderSharedPtr with the addresses
already set. Pass in a Envoy::Network::SocketAddressSetterImpl so that
we can continue setting the remote address for tests.

Signed-off-by: Jakub Sobon <mumak@google.com>
@mum4k mum4k requested a review from oschaaf January 12, 2021 08:07
@mum4k mum4k added the waiting-for-review A PR waiting for a review. label Jan 12, 2021
@mum4k mum4k requested a review from qqustc January 12, 2021 08:08
@mum4k
Copy link
Collaborator Author

mum4k commented Jan 12, 2021

@qqustc please review and assign back to me when done.

Copy link
Member

@oschaaf oschaaf left a comment

Choose a reason for hiding this comment

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

The code changes LGTM, but checking I noticed that Envoy's https://github.com/envoyproxy/envoy/blob/master/.bazelversion and https://github.com/envoyproxy/envoy/blob/master/.bazelrc have fairly recent changes.
Do we want to mirror those in this update?

@oschaaf oschaaf added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Jan 12, 2021
Signed-off-by: Jakub Sobon <mumak@google.com>
@mum4k
Copy link
Collaborator Author

mum4k commented Jan 12, 2021

Thanks for pointing that out @oschaaf, copied .bazelversion and .bazelrc from Envoy.

oschaaf
oschaaf previously approved these changes Jan 12, 2021
Copy link
Member

@oschaaf oschaaf left a comment

Choose a reason for hiding this comment

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

LGMT modulo my last remark

@oschaaf oschaaf added waiting-for-review A PR waiting for a review. waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. waiting-for-review A PR waiting for a review. labels Jan 12, 2021
qqustc
qqustc previously approved these changes Jan 13, 2021
Signed-off-by: Jakub Sobon <mumak@google.com>
@mum4k mum4k dismissed stale reviews from qqustc and oschaaf via 69369fb January 13, 2021 03:00
@mum4k
Copy link
Collaborator Author

mum4k commented Jan 13, 2021

@oschaaf please take another look and approve when ready.

@mum4k mum4k added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Jan 13, 2021
Copy link
Member

@oschaaf oschaaf left a comment

Choose a reason for hiding this comment

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

LGTM

@mum4k mum4k merged commit e6f9af7 into envoyproxy:master Jan 13, 2021
@mum4k mum4k deleted the envoy-update branch January 13, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-review A PR waiting for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants