Add a couple of missing fields to StreamInfoImpl::setFrom#25864
Add a couple of missing fields to StreamInfoImpl::setFrom#25864ravenblackx merged 1 commit intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Paul Sohn <paulsohn@google.com>
|
/assign @danzh2010 |
| attempt_count_ = info.attemptCount(); | ||
| upstream_bytes_meter_ = info.getUpstreamBytesMeter(); | ||
| bytes_sent_ = info.bytesSent(); | ||
| is_shadow_ = info.isShadow(); |
There was a problem hiding this comment.
Can we add some check or at least a comment in StreamInfo declaration about changing this place if new field is added?
There was a problem hiding this comment.
I am curious why the asserts in assertStreamInfoSize() of StreamInfoImplTest are not triggered then?
Is this because you fixed the asserts when you added them initially but forgot to add the copies in setFrom?
There was a problem hiding this comment.
@danzh2010, as tyxia pointed out, assertStreamInfoSize is the intended mechanism for keeping this up to date if new fields are added.
@tyxia, that's right -- these fields were already there in streaminfo, but I didn't add them to this method initially.
| attempt_count_ = info.attemptCount(); | ||
| upstream_bytes_meter_ = info.getUpstreamBytesMeter(); | ||
| bytes_sent_ = info.bytesSent(); | ||
| is_shadow_ = info.isShadow(); |
|
@RyanTheOptimist any chance you could merge this? |
|
or @alyssawilk? |
You'll probably want to use "/assign" to make sure PRs actually end up assigned. GitHub it so verbose with emails that it's easy to miss comments like this. sigh GitHub shake head The change LGTM, but it needs cross-company review as well. I'll assign it to someone for that review. |
|
/assign @ravenblackx |
5 participants
These were missed in #23648.
Commit Message: Add a couple of missing fields to StreamInfoImpl::setFrom
Risk Level: Low
Testing: Unit tested.
Docs Changes: N/A