stats: remove symbol table interface #19597
Conversation
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…ard decls) to impl. Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…kes sense in its new context. Signed-off-by: Joshua Marantz <jmarantz@google.com>
pradeepcrao
left a comment
There was a problem hiding this comment.
Could you also please update source/docs/stats.md?
| * same string is re-encoded, it may or may not encode to the same underlying | ||
| * symbol. | ||
| */ | ||
| class SymbolTableImpl : public SymbolTable { |
There was a problem hiding this comment.
Also change the names of files?
Also, if this class is not expected to be mocked, does it make sense declaring it as final?
There was a problem hiding this comment.
I added 'final' - though that's not something that happens much in Envoy C++ code for other classes that are not expected to be derived from.
I'm wondering if you think the compiler might be able to optimize better with this keyword in this case (no virtual anything), or whether this is just for the human reader. Either way I'm fine.
There was a problem hiding this comment.
It was more to validate that we indeed don't need an interface for the class, and this is the one and only implementation.
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
LGTM (pending file name changes) |
|
/assign-from @envoyproxy/senior-maintainers |
|
@envoyproxy/senior-maintainers assignee is @zuercher |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
.bazelversion
Outdated
| @@ -1 +1 @@ | |||
| 5.0.0 | |||
| 4.2.1 | |||
There was a problem hiding this comment.
oops I'll get rid of that. not sure what happened.
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
RE envoy/stats/symbol_table.h right now there are a lot of references to that throughout the codebase. I'll do a follow-up to move the forward decls into stats.h, but that would make this PR a bit too big if I do it now. |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…istent (#19788) Commit Message: follow-up to #19597 -- removes trivial forward decls from envoy/stats/symbol_table.h and renames all the files to reflect this is no longer an interface/impl pattern but a single implementation. This PR has a lot of files but all the changes are renames of includes except for the removal of envoy/stats/symbol_table.h and the moving of its forward decls into envoy/stats/stats.h. Additional Description: Risk Level: llw Testing: //test/... Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Joshua Marantz <jmarantz@google.com>
Commit Message: Removes the pure interface for symbol tables, as there is now only one implementation. It was originally added to enable a fake implementation while the impl and its usage was being refined, but now the interface layer just gets in the way. For example it's not possible to add a virtual template method to SymbolTable which would have been useful in envoyproxy#19563 Additional Description: Risk Level: low Testing: //test/... Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Joshua Marantz <jmarantz@google.com> Signed-off-by: Josh Perry <josh.perry@mx.com>
…istent (envoyproxy#19788) Commit Message: follow-up to envoyproxy#19597 -- removes trivial forward decls from envoy/stats/symbol_table.h and renames all the files to reflect this is no longer an interface/impl pattern but a single implementation. This PR has a lot of files but all the changes are renames of includes except for the removal of envoy/stats/symbol_table.h and the moving of its forward decls into envoy/stats/stats.h. Additional Description: Risk Level: llw Testing: //test/... Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Joshua Marantz <jmarantz@google.com> Signed-off-by: Josh Perry <josh.perry@mx.com>
Commit Message: Removes the pure interface for symbol tables, as there is now only one implementation. It was originally added to enable a fake implementation while the impl and its usage was being refined, but now the interface layer just gets in the way. For example it's not possible to add a virtual template method to SymbolTable which would have been useful in #19563
Additional Description:
Risk Level: low
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a