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

Move Factories to otelcol, delete otelcoltest #6792

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Dec 13, 2022

Resolves #6766

NopFactories is moved to otelcoltest. However, otelcol has its own private implementation in order to avoid a dependency cycle.

@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Base: 90.12% // Head: 90.12% // No change to project coverage 👍

Coverage data is based on head (230c661) compared to base (7ee940d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6792   +/-   ##
=======================================
  Coverage   90.12%   90.12%           
=======================================
  Files         250      250           
  Lines       14333    14333           
=======================================
  Hits        12918    12918           
  Misses       1165     1165           
  Partials      250      250           
Impacted Files Coverage Δ
component/componenttest/nop_factories.go 0.00% <ø> (ø)
otelcol/collector.go 77.94% <ø> (ø)
otelcol/configprovider.go 78.57% <100.00%> (ø)
otelcol/otelcoltest/config.go 76.00% <100.00%> (ø)
otelcol/unmarshaler.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@djaglowski djaglowski marked this pull request as ready for review December 13, 2022 20:46
@djaglowski djaglowski requested review from a team and bogdandrutu December 13, 2022 20:46
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

I was expecting some deletions in the service package of the things you just added.

@djaglowski
Copy link
Member Author

service.Factories and servicetest.NopFactories were (re)moved. What else did you expect to see removed from service?

@bogdandrutu
Copy link
Member

Where?

Screen Shot 2022-12-13 at 1 10 57 PM

@bogdandrutu
Copy link
Member

These are all the files in this PR (at least this is what I see)

Screen Shot 2022-12-13 at 1 13 27 PM

@djaglowski
Copy link
Member Author

These files were moved from service / servicetest.

image

@bogdandrutu bogdandrutu merged commit 1e6c7e5 into open-telemetry:main Dec 13, 2022
@djaglowski djaglowski deleted the mv-factories-otelcol branch December 13, 2022 22:27
jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
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.

Proper division between "service" & "otelcol" packages
2 participants