-
Notifications
You must be signed in to change notification settings - Fork 319
Cleanup classpath for modules with dependency on Groovy and Spock #10069
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
base: master
Are you sure you want to change the base?
Conversation
| addTestSuiteForDir('latestDepTest', 'test') | ||
|
|
||
| dependencies { | ||
| testImplementation(libs.javaparser) |
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.
just wondering -- do you know why this previously didn't need to be included?
I see that you also included libs.javaparser with your changes in gradle/libs.versions.toml, but if I understand correctly, this library wasn't a part of groovy-all
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.
Actually it is from groovy-all:
./gradlew :dd-java-agent:instrumentation:java:java-lang:java-lang-15.0:dependencies
Part of the tree:
org.codehaus.groovy:groovy-all:3.0.24
| +--- org.codehaus.groovy:groovy-groovydoc:3.0.24
| | +--- org.codehaus.groovy:groovy:3.0.24
| | \--- com.github.javaparser:javaparser-core:3.25.6
And there usage in tests:
import com.github.javaparser.utils.StringEscapeUtils
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.
This PR make that obvious, that we NEVER used groovydoc, but its transitive 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.
Ohhh cool okay, thanks!
What Does This Do
groovy-alldependency with individualgroovy-*modules.GroovyandSpockdependencies so they are used only within the test scope.Motivation
Using the bundled
groovy-allartifact introduces a large set of transitive dependencies into the classpath, many of which are unnecessary. Splitting it into specificgroovy-*modules helps reduce classpath footprint and improves build hygiene.Additionally, several modules previously declared
GroovyandSpockasapidependencies, even when they were only required for testing. This PR corrects that by limiting their usage to the appropriate scope.Additional Notes
Approximately 95% of modules already use
GroovyandSpockviatestImplementation. The remaining modules have been adjusted manually to prevent unneeded classpath pollution.