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

[chore] Split mdatagen configs in different files #11519

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

bogdandrutu
Copy link
Member

No description provided.

@bogdandrutu bogdandrutu requested a review from a team as a code owner October 23, 2024 03:29
@bogdandrutu bogdandrutu force-pushed the chore-metadata-config branch 4 times, most recently from 56003ef to 2b592c6 Compare October 23, 2024 03:38
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 78.76712% with 62 lines in your changes missing coverage. Please review.

Project coverage is 91.36%. Comparing base (2418583) to head (bb6316e).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/mdatagen/internal/metadata.go 66.87% 49 Missing and 3 partials ⚠️
cmd/mdatagen/internal/metric.go 86.48% 9 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11519      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.02%     
==========================================
  Files         433      434       +1     
  Lines       23659    23659              
==========================================
- Hits        21620    21616       -4     
- Misses       1663     1666       +3     
- Partials      376      377       +1     

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

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu bogdandrutu force-pushed the chore-metadata-config branch from 2b592c6 to bb6316e Compare October 23, 2024 11:57
@bogdandrutu bogdandrutu merged commit 41f8779 into open-telemetry:main Oct 23, 2024
48 of 49 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 23, 2024
Comment on lines +52 to +55
if md.Status != nil {
// status is not required for subcomponents.
errs = errors.Join(errs, errors.New("status must be empty for subcomponents"))
}
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this error condition was not present on the original file, and it is making the contrib release fail. Part of the solution is #11167, but could we also have these kinds of changes listed in the PR description (and maybe changelog as well)? It was a bit hard for me to debug this (since the PR title talks about just moving files around, I discarded this PR as the cause of the error initially)

Copy link
Contributor

Choose a reason for hiding this comment

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

I vote to reset back to the original logic, and have an issue if we want to evolve the logic.

djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this pull request Nov 21, 2024
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this pull request Dec 19, 2024
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

4 participants