Adding Java provenance info to gradle-based builds #63
Conversation
| javaConvention.getSourceSets().all(sourceSet -> { | ||
| sourceSets.add(sourceSet); | ||
| //Collect Java metadata for each project (used for Java Provenance) | ||
| JavaPluginExtension javaPluginExtension = project.getExtensions().getByType(JavaPluginExtension.class); |
There was a problem hiding this comment.
JavaPluginExtension was added in Gradle 4.10, which is newer than the oldest version we currently support (4.7), and we were talking about going back to support even earlier versions of Gradle. Ultimately to support Gradle versions new and old we'll need to use reflection to load one or the other at runtime. For now stick with JavaPluginConvention if you can.
| private Method sourceFileGetMarkersMethod() { | ||
| if (sourceFileGetMarkers == null) { | ||
| try { | ||
| Class<?> sourceFileClass = getClassLoader().loadClass("org.openrewrite.SourceFile"); | ||
| sourceFileGetMarkers = sourceFileClass.getMethod("getMarkers"); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| return sourceFileGetMarkers; | ||
| } |
There was a problem hiding this comment.
Why isn't this RewriteReflectiveFacade.SourceFile.getMarkers()? Same question re: sourceFileWithMarkersMethod() and markersAddIfAbsentMethod().
This is inconsistent with how the rest of the facade is laid out and I don't see why that should be.
There was a problem hiding this comment.
I honestly was trying to avoid creating:
A facade for Markers, and a polymorphic facade for Marker and then having JavaProvenance extend that.
When in fact we know that there is a very limited use case for adding the JavaProvenance
| .vmRuntimeVersion(javaRuntimeVersion) | ||
| .vmRuntimeVersion(System.getProperty("java.vm.vendor")) |
There was a problem hiding this comment.
This looks accidental. You probably didn't mean to set the same property twice with different values
There was a problem hiding this comment.
yep, should be vmVendor, good catch
Fixes #62