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

Add new messages for topic statistics #98

Merged
merged 3 commits into from
Apr 17, 2020

Conversation

prajakta-gokhale
Copy link

@prajakta-gokhale prajakta-gokhale commented Apr 16, 2020

Add new messages required for topic statistics.

Prajakta Gokhale added 2 commits April 16, 2020 15:15
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
@prajakta-gokhale
Copy link
Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dabonnie
Copy link

@dirk-thomas re-review please?

# Constant for uninitialized
uint8 STATISTICS_DATA_TYPE_UNINITIALIZED = 0

# Allowed values
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to check the generated code on how these comments appear in docblocks. I would guess this one will be associated with the first enum which seems weird.

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

My comment about the "comments" doesn't need to block the merge. It can be followed up on afterwards if necessary.

@dabonnie
Copy link

My comment about the "comments" doesn't need to block the merge. It can be followed up on afterwards if necessary.

Sounds good. Who can perform the merge?

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.

4 participants