Skip to content

Stats interface atomization#4071

Merged
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
jmarantz:stats-impl-atomization
Aug 7, 2018
Merged

Stats interface atomization#4071
mattklein123 merged 6 commits intoenvoyproxy:masterfrom
jmarantz:stats-impl-atomization

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz commented Aug 7, 2018

Description: envoy/stats/stats.h was a collection of many loosely related class interfaces. This PR breaks them up, mostly one class per file, except for a few closely related ones. One benefit of this is that code changes can be made much more quickly, without recompiling the world on every edit. E.g. many uses of Stats only need Scope or StatsOptions, and don't need many of the other classes that change more frequently. Another benefit is simply to offer a better parallel to the implementation breakup already merged.

Risk Level: low
Testing: //test/...
Docs Changes: N/A
Release Notes: N/A

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>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz requested review from lizan and zuercher as code owners August 7, 2018 01:16
envoy_cc_library(
name = "stats_interface",
hdrs = ["stats.h"],
hdrs = [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can BUILD targets be split to multiple ones?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I was thinking of doing that in a follow-up as that will be another large PR.

…uild rules.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Epic!

@mattklein123 mattklein123 merged commit c68063c into envoyproxy:master Aug 7, 2018
@jmarantz jmarantz deleted the stats-impl-atomization branch August 7, 2018 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants