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.

Sign up for GitHub

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

Jump to bottom

Change metrics enum values to follow protobuf style #196

Merged
merged 3 commits into from
Aug 10, 2020
Merged

Change metrics enum values to follow protobuf style #196

merged 3 commits into from
Aug 10, 2020

Conversation

bogdandrutu
Copy link
Member

@bogdandrutu bogdandrutu commented Aug 6, 2020 •
edited
Loading

Following the style guide from https://developers.google.com/protocol-buffers/docs/style#enums
There is a small caveat in golang golang/protobuf#513

Updates #131

@bogdandrutu bogdandrutu requested review from a team August 6, 2020 21:17
yurishkuro
yurishkuro approved these changes Aug 6, 2020
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Are there similar missing UNSPECIFIED values in the other enums, e.g. in traces proto?

@bogdandrutu
Copy link
Member Author

@yurishkuro creating a PR per directory, we have different approvals so want to give them a chance to comment :)

@bogdandrutu bogdandrutu changed the title Change enum values to follow protobuf style Change metrics enum values to follow protobuf style Aug 6, 2020
tigrannajaryan
tigrannajaryan approved these changes Aug 10, 2020
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM

@bogdandrutu bogdandrutu merged commit 78ecc42 into open-telemetry:master Aug 10, 2020
@bogdandrutu bogdandrutu deleted the enummetrics branch August 10, 2020 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@MrAlias MrAlias MrAlias approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bogdandrutu @yurishkuro @tigrannajaryan @MrAlias