Refactor NullRouteImpl out of async client #28638
Conversation
|
/retest |
1 similar comment
|
/retest |
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @alyssawilk |
source/common/http/BUILD
Outdated
| hdrs = ["async_client_impl.h"], | ||
| srcs = [ | ||
| "async_client_impl.cc", | ||
| "null_route_impl.cc", |
There was a problem hiding this comment.
Please create a separate target for the null_route_impl.cc library, since it is canonical approach with bazel.
There was a problem hiding this comment.
done, thanks!
Signed-off-by: Vikas Choudhary <choudharyvikas16@gmail.com>
There was a problem hiding this comment.
Is this file still needed? Looks like it can be removed.
There was a problem hiding this comment.
it is still being used in NullRoute here, https://github.com/envoyproxy/envoy/pull/28638/files/3bfa74da7c8078d4b6915c263fd17aa5b46a3608#diff-d7f483e1142b03905f8668f2f86fcc9c68c60a7b1ce04c2360c902bd6192bf14R207-R208 . Sorry I could not get how this can be removed.
There was a problem hiding this comment.
I see. All it does is just including the "envoy/extensions/early_data/v3/default_early_data_policy.pb.h". Can you just include the "envoy/extensions/early_data/v3/default_early_data_policy.pb.h" where the default_early_data_policy.h is included and then get rid of the default_early_data_policy.h ?
There was a problem hiding this comment.
std::unique_ptr<Router::EarlyDataPolicy> early_data_policy_{
new envoy::extensions::early_data::v3::DefaultEarlyDataPolicy()};
EarlyDataPolicy is an abstract class which is implemented by DefaultEarlyDataPolicy. default_early_data_policy.pb.h cannot give us an instance of Router::EarlyDataPolicy.
Also I want to avoid making any functional changes in this PR.
There was a problem hiding this comment.
Oh sorry, I have missed that there is code below and thought the file was empty.
|
Ping @yanavlasov |
1 similar comment
|
Ping @yanavlasov |
|
LGTM otherwise /wait |
Commit Message: Refactor NullRouteImpl out of async client.
Additional Description:
Risk Level: low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]