-
Notifications
You must be signed in to change notification settings - Fork 835
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
Stop publishing test fixtures with opentelemetry-api #6695
Stop publishing test fixtures with opentelemetry-api #6695
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6695 +/- ##
=========================================
Coverage 90.09% 90.09%
Complexity 6390 6390
=========================================
Files 711 711
Lines 19333 19333
Branches 1891 1891
=========================================
Hits 17418 17418
Misses 1335 1335
Partials 580 580 ☔ View full report in Codecov by Sentry. |
@@ -1,7 +1,6 @@ | |||
plugins { | |||
id("otel.java-conventions") | |||
id("otel.publish-conventions") | |||
id("java-test-fixtures") |
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.
In hindsight, adding java-test-fixtures
was overly hasty, since as shown in #6693 it impacts the pom.xml
and publishes additional artifacts we didn't intend / account for. When we need to share test code in this repository, we simply add a *:testing-internal
module and omit the otel.publish-conventions
. I suspect this pattern was selected in the past after investigating test fixtures and seeing these types of side affects.
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.
FYI @zeitlinger
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.
could we have detected this before merging?
Resolves #6693.
Will need to publish in 1.42.1 patch release.