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

proto/openmetrics_data_model.proto: Don't assign field number twice #181

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Dec 14, 2020

Compiling proto/openmetrics_data_model.proto with libprotoc v3.6.1 fails
with:

protoc failed: open_metrics.proto:198: 18: Field number 2 has already been used in \"openmetrics.SummaryValue\" by field \"int_value\".\n"

In the SummaryValue message the field number 2 is used twice, once for
int_value and once for count. With this commit, all field numbers within
SummaryValue are monotonically increasing instead.

As far as I can tell this never compiled in the first place, thus backward
compatibility should not be an issue.

Compiling `proto/openmetrics_data_model.proto` with `libprotoc` `v3.6.1` fails
with:

> `protoc failed: open_metrics.proto:198: 18: Field number 2 has already been
  used in \"openmetrics.SummaryValue\" by field \"int_value\".\n"`

In the `SummaryValue` `message` the field number `2` is used twice, once for
`int_value` and once for `count`. With this commit, all field numbers within
`SummaryValue` are monotonically increasing instead.
@mxinden
Copy link
Member Author

mxinden commented Dec 14, 2020

For what I can tell the bug was introduced with #151.

@ad-m
Copy link

ad-m commented Dec 17, 2020

Can we apply linting to that?

Copy link
Contributor

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the PR!

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