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 shaded testing dependencies to internal package. #3305

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

anuraaga
Copy link
Contributor

The frameworks we use for HTTP in testing is an implementation detail of our testing-common libraries and our repo's tests, they shouldn't be used directly by users. It's easy enough to bring in a favorite HTTP framework if a downstream repo needs it I think.

I tried moving the dependency from api to implementation at first but remembered Groovy doesn't let you compile a subclass without the parent's dependencies on the compile classpath (grr).

import io.opentelemetry.testing.armeria.client.WebClient
import io.opentelemetry.testing.internal.armeria.client.WebClient
Copy link
Member

Choose a reason for hiding this comment

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

what's the purpose of moving these to internal package if they still have external usages? or is this a first step?

Copy link
Contributor Author

@anuraaga anuraaga Jun 15, 2021

Choose a reason for hiding this comment

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

By external usage do you mean the usages in this repo? internal is fine to use in this repo, similar to the internal package being used across artifacts in the opentelemetry-java repo. The purpose is to make them unsupported for use from consumers of the testing-common package from Maven Central

@iNikem iNikem merged commit 7ad9e7a into open-telemetry:main Jun 15, 2021
@trask trask mentioned this pull request Jun 16, 2021
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