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

mdatagen: support custom package name #11232

Merged
merged 11 commits into from
Oct 10, 2024

Conversation

braydonk
Copy link
Contributor

@braydonk braydonk commented Sep 20, 2024

Description

This PR adds support for a new --package_name flag generated_package_name config field to allow mdatagen to generate packages with names other than metadata.

Link to tracking issue

Fixes #11231

Testing

  • Unit tests
  • go installing this branch and using it in a test scenario with two yaml files and generating two packages.

Documentation

Adds support for a new `--package_name` flag to allow mdatagen to
generate packages with names other than `metadata`.
@braydonk braydonk requested a review from a team as a code owner September 20, 2024 15:16
cmd/mdatagen/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Overall this looks pretty good, just a couple of comments, it looks like make gofmt needs to be run to sort out the import statements. Also please add a note in the schema file: https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/mdatagen/metadata-schema.yaml

@braydonk
Copy link
Contributor Author

braydonk commented Oct 9, 2024

Also please add a note in the schema file

Done in d6c50bf

I believe running make gofmt in 554391b addressed all other comments.

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.15%. Comparing base (68f0264) to head (b142b2b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/mdatagen/internal/command.go 60.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11232   +/-   ##
=======================================
  Coverage   92.15%   92.15%           
=======================================
  Files         432      432           
  Lines       20291    20293    +2     
=======================================
+ Hits        18700    18702    +2     
  Misses       1228     1228           
  Partials      363      363           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for the update @braydonk, looks good now, can merge it once the conflict has been resolved

@codeboten codeboten merged commit 8211777 into open-telemetry:main Oct 10, 2024
47 of 48 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 10, 2024
braydonk added a commit to braydonk/opentelemetry-collector that referenced this pull request Oct 16, 2024
I noticed a mistake in my previous PR open-telemetry#11232; some function calls did
not pass the correct package name in (passing in "metadata" instead of
the intended generated package name). This PR attempts to address the
potential for this mistake to even occur by providing a wrapper
`generateFile` function that automatically uses the generated package
name from the metadata. The original version of the function that
accepts a package name is intact for the templates that are going in the
base package instead of the generated one.
braydonk added a commit to braydonk/opentelemetry-collector that referenced this pull request Oct 16, 2024
I noticed a mistake in my previous PR open-telemetry#11232; some function calls did
not pass the correct package name in (passing in "metadata" instead of
the intended generated package name). This PR attempts to address the
potential for this mistake to even occur by providing a wrapper
`generateFile` function that automatically uses the generated package
name from the metadata. The original version of the function that
accepts a package name is intact for the templates that are going in the
base package instead of the generated one.
djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this pull request Nov 21, 2024
#### Description
This PR adds support for a new ~~`--package_name` flag~~
`generated_package_name` config field to allow mdatagen to generate
packages with names other than `metadata`.

#### Link to tracking issue
Fixes open-telemetry#11231 

#### Testing
* Unit tests
* `go install`ing this branch and using it in a test scenario with two
yaml files and generating two packages.

#### Documentation
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.

Allow mdatagen to generate packages with names other than metadata
4 participants