-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
stream_info: Make some struct members private #34058
Conversation
Signed-off-by: Ali Beyad <abeyad@google.com>
Why close? I think this is a good fix. |
Hey @kyessenov thanks! I ran into some issues w/ the PR and wasn't sure when I would be able to get back to it, so closed it, but I just tried a fix so re-opening the PR now. |
@kyessenov ping |
absl::InlinedVector<ResponseFlag, 4> response_flags_{}; | ||
bool health_check_request_{}; | ||
Router::RouteConstSharedPtr route_; | ||
envoy::config::core::v3::Metadata metadata_{}; | ||
FilterStateSharedPtr filter_state_; | ||
|
||
private: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep the order of the fields the same? This is a fairly common data structure and the layout would change if we re-order - possibly for worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests have a size check which passed:
envoy/test/common/stream_info/stream_info_impl_test.cc
Lines 44 to 48 in c6b8553
sizeof(stream_info) == 840 || sizeof(stream_info) == 856 || sizeof(stream_info) == 888 || | |
sizeof(stream_info) == 776 || sizeof(stream_info) == 728 || sizeof(stream_info) == 744 || | |
sizeof(stream_info) == 680 || sizeof(stream_info) == 696 || sizeof(stream_info) == 688 || | |
sizeof(stream_info) == 736 || sizeof(stream_info) == 728 || sizeof(stream_info) == 712 || | |
sizeof(stream_info) == 704) |
Signed-off-by: Ali Beyad <abeyad@google.com>
/retest |
No description provided.