-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add missing buildscript classpath configuration #4459
Conversation
rewrite-gradle/src/main/java/org/openrewrite/gradle/marker/GradleBuildscript.java
Show resolved
Hide resolved
5a57b66
to
8b57a69
Compare
@sambsnyd @shanman190 I am getting failing tests on main since this merge. All have the same cause
|
@sihutch, do you by chance have some old rewrite snapshots published to the |
What's changed?
Now capturing the buildscript
classpath
dependency configuration.What's your motivation?
I was reminded that this configuration was presently not being captured during the review of #4376, so this means that the
GradleDependency
trait fails to match buildscript dependencies.Anything in particular you'd like reviewers to focus on?
I've moved plugin repositories into the
GradleBuildscript
nested object. The thought is that this more closely aligns the purposes of the repositories from a code discoverability standpoint and the expectation is for the new style to be non-null. Given current serialized LSTs, there's an internal null check to help make sure to avoid some of the possibleNullPointerException
cases.Anyone you would like to review specifically?
@sambsnyd
Have you considered any alternatives or workarounds?
classpath
configuration to the existingnameToConfiguration
map, but presently this is tracking project configurations. So by doing so we'd be blending the buildscript and project configuration together which is probably not desirable.GradleDependencyConfiguration
directly on the marker objects themselves to capture the same information, but again we're now capturing more information about the buildscript itself and having to use the naming conventions to differentiate between the buildscript settings and the project settings.Any additional context
I would have liked to go ahead and drop the
mavenPluginRepositories
altogether, but I suspected that previously serialized LSTs could cause us some grief.openrewrite/rewrite-gradle-tooling-model#23
Checklist