-
Notifications
You must be signed in to change notification settings - Fork 889
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
Have consistent formatting for semantic convention enums #1598
Conversation
brief: "Apple Darwin" | ||
- id: FREEBSD | ||
value: 'FREEBSD' | ||
- id: freebsd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what to do with BSDs tbh. free_bsd or free_b_s_d would be impossibly difficult to read. That's why I'm keeping it simple here by keeping them as a single word.
Currently, semantic convention enum values are not consistent. For example, os.type values are all capitals whereas cloud.infrastructure_service values are underscored lowercase. This change improves the inconsistencies. We also expect language implementations to autogenerate code from enum values. Each language has their own conventions for constant variable identifiers and we expect the consistent formatting is going to help the language implementors. Fixes #1519.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Probably we should also add this to some guidelines section, so future conventions stay uniform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we already have scripts reading these YAML files. I suggest adding a check that all IDs are lowercase (and also ASCII, alphanum, no dashes, whichever rules we want to enforce). It's better than just a written convention, and checks should fail the build on violations.
cc @thisthat |
Submitted https://github.com/open-telemetry/opentelemetry-specification/issues/1603 to add linting rules for enum values. |
Adding such a check in our tool would be easy :) I am happy to work on that as soon as we agree on the rule set :) |
Ready to be merged now. |
@alolita FYI, We have to decided to merge this PR right after we do (today) our monthly release. |
…try#1598) * Have consistent formatting for semantic convention enums Currently, semantic convention enum values are not consistent. For example, os.type values are all capitals whereas cloud.infrastructure_service values are underscored lowercase. This change improves the inconsistencies. We also expect language implementations to autogenerate code from enum values. Each language has their own conventions for constant variable identifiers and we expect the consistent formatting is going to help the language implementors. Fixes open-telemetry#1519.
Currently, semantic convention enum values are not consistent.
For example, os.type values are all capitals whereas
cloud.infrastructure_service values are underscored lowercase.
This change improves the inconsistencies.
We also expect language implementations to autogenerate code
from enum values. Each language has their own conventions for
constant variable identifiers and we expect the consistent
formatting is going to help the language implementors.
Fixes #1519.