mbien commented on code in PR #8810:
URL: https://github.com/apache/netbeans/pull/8810#discussion_r2386406024
##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -1362,18 +1391,22 @@ private void detectArtifacts(NbProjectInfoModel model) {
}
}
Map<String, Object> archives = new HashMap<>();
Review Comment:
nitpick: the map could be changed to `<String, File>` which would remove an
"unnecessary cast" warning.
##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -203,6 +202,28 @@ class NbProjectInfoBuilder {
private static final GradleVersion GRADLE_VERSION =
GradleVersion.current().getBaseVersion();
+ /** Jar.getClassifier() was deprecated since Gradle 5.2, removed in 8.0 */
+ private static final Method GRADLE_JAR_GET_CLASSIFIER;
+
+ /** Jar.getArchivePath() was deprecated since Gradle 5.2, removed in 8.0 */
+ private static final Method GRADLE_JAR_GET_ARCHIVEPATH;
+
+ static {
+ Method getClassifier = null;
+ Method getArchivePath = null;
+ try {
+ getClassifier = Jar.class.getMethod("getClassifier");
+ getArchivePath = Jar.class.getMethod("getArchivePath");
+ } catch (ReflectiveOperationException e) {
+ if (GRADLE_VERSION.compareTo(GradleVersion.version("8.0")) < 0) {
+ LOG.error("Did not find expected method(s) on {}", Jar.class,
e);
+ }
+ // else: expected on newer versions, so stay silent
+ }
Review Comment:
i would flip this inside out:
```java
if (GRADLE_VERSION.compareTo(GradleVersion.version("8.0")) < 0) {
try {
getClassifier = Jar.class.getMethod("getClassifier");
getArchivePath = Jar.class.getMethod("getArchivePath");
} catch (NoSuchMethodException e) {
LOG.error("Did not find expected method(s) on {}",
Jar.class, e);
}
}
// null on newer versions
```
no need to swallow exceptions on newer gradle versions imo. (they would show
up in JFR logs too)
##########
extide/gradle/netbeans-gradle-tooling/src/main/java/org/netbeans/modules/gradle/tooling/NbProjectInfoBuilder.java:
##########
@@ -1362,18 +1391,22 @@ private void detectArtifacts(NbProjectInfoModel model) {
}
}
Map<String, Object> archives = new HashMap<>();
- beforeGradle("5.2", () -> {
- // The jar.getCassifier() and jar.getArchievePath() are deprecated
since 5.2
- // These methods got removed in 8.0
- project.getTasks().withType(Jar.class).forEach(jar -> {
- archives.put(jar.getClassifier(), jar.getArchivePath());
- });
- });
- sinceGradle("5.2", () -> {
- project.getTasks().withType(Jar.class).forEach(jar -> {
- archives.put(jar.getArchiveClassifier().get(),
jar.getDestinationDirectory().file(jar.getArchiveFileName().get()).get().getAsFile());
- });
- });
+ Consumer<Jar> jarToArchivesClassifierAndPath =
+ sinceGradleOrDefault(
+ "5.2",
+ () -> jar -> archives.put(jar.getArchiveClassifier().get(),
jar.getDestinationDirectory().file(jar.getArchiveFileName().get()).get().getAsFile()),
+ () -> {
+ return jar -> {
+ try {
+ archives.put(
+ (String) GRADLE_JAR_GET_CLASSIFIER.invoke(jar),
+ (File) GRADLE_JAR_GET_ARCHIVEPATH.invoke(jar));
+ } catch (ReflectiveOperationException e) {
+ sneakyThrow(e);
+ }
+ };
+ });
Review Comment:
nitpick:
```java
() -> jar -> {
try {
archives.put(
(String)
GRADLE_JAR_GET_CLASSIFIER.invoke(jar),
(File)
GRADLE_JAR_GET_ARCHIVEPATH.invoke(jar));
} catch (ReflectiveOperationException e) {
sneakyThrow(e);
}
}
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
