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

Have consistent formatting for semantic convention enums #1598

Merged
merged 6 commits into from
Apr 15, 2021
Merged

Have consistent formatting for semantic convention enums #1598

merged 6 commits into from
Apr 15, 2021

Conversation

rakyll
Copy link
Contributor

@rakyll rakyll commented Apr 6, 2021

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.

@rakyll rakyll requested review from a team April 6, 2021 17:18
brief: "Apple Darwin"
- id: FREEBSD
value: 'FREEBSD'
- id: freebsd
Copy link
Contributor Author

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.
@rakyll rakyll requested review from a team April 6, 2021 17:21
Copy link
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM.

@carlosalberto
Copy link
Contributor

Probably we should also add this to some guidelines section, so future conventions stay uniform.

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.

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.

@carlosalberto
Copy link
Contributor

cc @thisthat

@rakyll
Copy link
Contributor Author

rakyll commented Apr 6, 2021

Submitted https://github.com/open-telemetry/opentelemetry-specification/issues/1603 to add linting rules for enum values.

@thisthat
Copy link
Member

thisthat commented Apr 7, 2021

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 :)

@rakyll
Copy link
Contributor Author

rakyll commented Apr 13, 2021

Ready to be merged now.

@carlosalberto
Copy link
Contributor

@alolita FYI, We have to decided to merge this PR right after we do (today) our monthly release.

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.

Consistently format semantic convention enums
8 participants