Conversation
Signed-off-by: GitHub Action <noreply@github.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Co-authored-by: Ryan Hamilton <rch@google.com> Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim
force-pushed
the
update-envoy-1667829915
branch
from
8812392 to
ae7beb4
Compare
| absl::StrAppend(&output, it.first.length(), "\n", it.first, it.second.length(), "\n", | ||
| it.second); | ||
| for (const auto& [key, value] : store_) { | ||
| std::string string = value.first; |
There was a problem hiding this comment.
Maybe we could come up with a more descriptive name in here? string doesn't say much. I find it hard to read this code as is but I am not familiar with it so it may be because of that.
There was a problem hiding this comment.
I'd love for this to be a more descriptive name, but ultimately the store_ is a key-value store, so you get the key and value out of it, but the value is a std::pair<std::string, std::optional<std::chrono::duration<long long>>>, so it has two members: first is the actual string value, which could be anything as long as it's a string, and second is an optional duration which is its TTI.
So short of calling it string_value I'm not sure how else I could call this.
Thoughts?
There was a problem hiding this comment.
Thank you for the explanation.
I think that string_value sounds better but that's not a blocking comment so leaving the decision up to you.
There was a problem hiding this comment.
Sorry about the unreadable interface here! I've sent out an upstream Envoy PR to make this more readable. envoyproxy/envoy#23904 (Though it obviously won't help with JP's current PR, but eventually :>)
| absl::StrAppend(&output, it.first.length(), "\n", it.first, it.second.length(), "\n", | ||
| it.second); | ||
| for (const auto& [key, value] : store_) { | ||
| std::string string = value.first; |
There was a problem hiding this comment.
Thank you for the explanation.
I think that string_value sounds better but that's not a blocking comment so leaving the decision up to you.
Signed-off-by: JP Simard <jp@jpsim.com>
…builder-function * origin/main: ci: hopefully fixing bes timeout failures (#2666) Update Envoy (#2660) bazel: update rules_jvm_external to 4.5 (#2665) Remove note about DWARF patch being required (#2645) Bump Lyft Support Rotation (#2661) build: remove alwayslink set enablePlatformCertificateValidation to false on iOS by default (#2663) bump Envoy dep (#2659) Implement an iOS platform certificate verifier. (#2638) Signed-off-by: JP Simard <jp@jpsim.com>
4 participants
Automated changes by create-pull-request GitHub action