Skip to content
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

Adds usage count msgs to support hip 118 visualization data #428

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

andymck
Copy link
Contributor

@andymck andymck commented Nov 7, 2024

No description provided.

src/service/poc_mobile.proto Outdated Show resolved Hide resolved
Comment on lines 15 to 20
uint64 service_provider_subscriber_count = 2;
// Number of unique discovery mapping enabled subscribers active
// in the hex during the epoch
uint64 disco_mapping_count = 3;
// Number of unique offload users active in the hex during the epoch
uint64 offload_count = 4;
Copy link

@dcyoung dcyoung Nov 7, 2024

Choose a reason for hiding this comment

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

if i understand correctly, these 3 fields represent quantities of users in various categories. Consider a common naming convention or suffix between the fields to indicate this... ie: xxxx_user_count or xxx_subscriber_count

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed here: 1b9d954

src/service/poc_mobile.proto Outdated Show resolved Hide resolved
src/service/poc_mobile.proto Show resolved Hide resolved
src/service/poc_mobile.proto Outdated Show resolved Hide resolved
src/service/poc_mobile.proto Show resolved Hide resolved
@dcyoung
Copy link

dcyoung commented Nov 11, 2024

@andymck looks great to me.

@andymck andymck marked this pull request as ready for review November 18, 2024 13:58
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.

2 participants