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
[WFLY-17237] Drop source transformation for opentelemetry subsystem #16250
Conversation
boms/legacy-ee/pom.xml
Outdated
<version>${full.maven.version}</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>${full.maven.groupId}</groupId> | ||
<artifactId>wildfly-opentelemetry-api-legacy-namespace</artifactId> | ||
<artifactId>wildfly-opentelemetry-api</artifactId> | ||
<version>${full.maven.version}</version> | ||
</dependency> |
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.
Both of these should just be deleted. Anything that needs the two non-legacy artifacts shouldn't be relying on this bom to manage that dep. And I don't think anything is doing that.
<scope>import</scope> | ||
</dependency> | ||
</dependencies> | ||
</dependencyManagement> |
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.
The observability/pom.xml has a dependencyManagement import of wildfly-legacy-ee-bom. That can either be deleted, and all the child modules control that, as you do here, or you can change that to wildfly-standard-ee-bom and drop this whole block.
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.
I tried removing that, but that breaks the micrometer build, and we trying to avoid changing that module as part of this work, though I guess it's safe/clean enough. I'll do that if you're ok with it.
observability/opentelemetry/pom.xml
Outdated
<scope>import</scope> | ||
</dependency> | ||
</dependencies> | ||
</dependencyManagement> |
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.
Same comment that I made in opentelemetry-api/pom.xml applies here.
Update dependencies for base extension Remove source transform modules Rename files Update imports
/retest |
Thanks @jasondlee |
https://issues.redhat.com/browse/WFLY-17237
Update dependencies for base extension
Remove source transform modules
Rename files
Update imports