gnodet-bot commented on code in PR #27932:
URL: https://github.com/apache/camel/pull/27932#discussion_r4290935721
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/catalog/CatalogBaseCommand.java:
##########
@@ -224,14 +224,19 @@ int sortRow(Row o1, Row o2) {
return o1.name.compareToIgnoreCase(o2.name) * negate;
case "level":
case "support-level":
- return o1.level.compareToIgnoreCase(o2.level) * negate;
+ return compareIgnoreCase(o1.level, o2.level) * negate;
case "description":
- return o1.description.compareToIgnoreCase(o2.description) *
negate;
+ return compareIgnoreCase(o1.description, o2.description) *
negate;
default:
return 0;
}
}
+ // a missing value sorts first
+ private static int compareIgnoreCase(String s1, String s2) {
Review Comment:
💡 **Duplication:** `compareIgnoreCase` is added identically in
`CatalogBaseCommand` (here) and `CatalogKamelet` (line 144). Consider
extracting to a shared utility (e.g. `StringHelper` or a catalog-specific
util). Not blocking.
##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesRun.java:
##########
@@ -801,28 +803,52 @@ private Integer deployProject(String workingDir, boolean
reload) throws Exceptio
pb.command(args.toArray(String[]::new));
- if (quiet || !verbose) {
- pb.redirectErrorStream(true);
- pb.redirectOutput(ProcessBuilder.Redirect.DISCARD);
- } else {
- pb.inheritIO(); // run in foreground (with IO so logs are visible)
- }
-
+ Path log = mavenOutput(pb);
Process p = pb.start();
// wait for that process to exit as we run in foreground
int exit = p.waitFor();
if (exit != 0) {
- String msg = "Deployment to %s failed!";
- if (!verbose) {
- msg += " (use --verbose for more details)";
- }
- printer().printErr(msg.formatted(clusterType));
+ printer().printErr("Deployment to %s
failed!".formatted(clusterType));
+ printMavenOutput(log);
return exit;
}
return 0;
}
+ /**
+ * Shows the output of Maven with --verbose, and keeps it in a log file
otherwise, for when it fails.
+ *
+ * @return the log file, or <tt>null</tt> with --verbose
+ */
+ private Path mavenOutput(ProcessBuilder pb) throws IOException {
+ if (!quiet && verbose) {
+ pb.inheritIO(); // run in foreground (with IO so logs are visible)
+ return null;
+ }
+ Path log = Files.createTempFile("camel-kubernetes-", ".log");
+ log.toFile().deleteOnExit();
Review Comment:
💡 **Temp file leaks during process lifetime:** `deleteOnExit()` only cleans
up at JVM shutdown. Both call sites return 0 on success without explicitly
deleting `log`. For repeated invocations or long sessions, these files
accumulate. Consider `Files.deleteIfExists(log)` on the success path.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/CommandLineHelper.java:
##########
@@ -201,6 +203,40 @@ private static Path getUserPropertyFile(boolean local) {
*
* @return the user home directory.
*/
+ /**
+ * The arguments of a command line typed as one text, such as
<tt>--jvm-args</tt>: split at whitespace outside
Review Comment:
⚠️ **Orphaned Javadoc:** The existing `getHomeDir()` Javadoc (`@return the
user home directory.`) is now orphaned — the new `splitArguments` method (with
its own `/** */` block) was inserted between the Javadoc and its target method.
Java attaches a Javadoc to the immediately following declaration, so
`getHomeDir()` loses its documentation. Move `splitArguments` (and its Javadoc)
*after* `getHomeDir()`, or add a standalone Javadoc before `getHomeDir()`.
--
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]
