squakez commented on code in PR #19497:
URL: https://github.com/apache/camel/pull/19497#discussion_r2417088585
##########
archetypes/camel-archetype-component/src/main/resources/archetype-resources/src/main/java/__name__Producer.java:
##########
@@ -31,7 +31,7 @@ public class ${name}Producer extends DefaultProducer {
}
public void process(Exchange exchange) throws Exception {
- System.out.println(exchange.getIn().getBody());
+ LOG.info(exchange.getIn().getBody().toString());
Review Comment:
This is an archetype for components. It would be fine to keep the default
output stream, but, given we do have LOG, better to make use of it.
##########
components/camel-jmx/src/test/java/org/apache/camel/component/jmx/XmlFixture.java:
##########
@@ -54,7 +59,9 @@ public static void dump(Source aActual)
TransformerFactory tf = TransformerFactory.newInstance();
Transformer transformer = tf.newTransformer();
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
- transformer.transform(aActual, new StreamResult(System.out));
+ StringWriter sw = new StringWriter();
Review Comment:
I think the entire method is not used any longer. You can try to remove it
completely so we don't either need the import.
##########
components/camel-jasypt/src/main/java/org/apache/camel/component/jasypt/Main.java:
##########
@@ -24,9 +24,13 @@
import org.jasypt.encryption.pbe.StandardPBEStringEncryptor;
import org.jasypt.iv.RandomIvGenerator;
import org.jasypt.salt.RandomSaltGenerator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class Main {
+ private static final Logger LOG = LoggerFactory.getLogger(Main.class);
Review Comment:
No, this one is a CLI, so we need to revert the standard output/error as it
was.
##########
components/camel-mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpServerConsumerTransactionTest.java:
##########
@@ -65,16 +66,7 @@ protected CamelContext createCamelContext() throws Exception
{
return context;
}
- @BindToRegistry("target")
Review Comment:
Why removing this?
##########
components/camel-oauth/src/test/java/org/apache/camel/test/oauth/AbstractOAuthCodeFlowTest.java:
##########
@@ -42,8 +46,8 @@ void testCodeFlowAuth() throws Exception {
Assertions.assertEquals(1,
keycloak.realm(KEYCLOAK_REALM).clients().findByClientId(TEST_CLIENT_ID).size());
Assertions.assertEquals(1,
keycloak.realm(KEYCLOAK_REALM).users().search("alice").size());
- System.out.println("✅ Keycloak realm, client, and user
available!");
- System.out.println("✅ Open: " + APP_BASE_URL);
+ LOG.info("✅ Keycloak realm, client, and user available!");
Review Comment:
Uh, those emoj smells AI gen output. I'd take the opportunity to remove them
as they may not necessarily print nice on CI logging output.
##########
components/camel-splunk/src/test/java/org/apache/camel/component/splunk/ProducerTest.java:
##########
@@ -79,7 +82,7 @@ public void setup() throws IOException {
when(inputCollection.get(anyString())).thenReturn(input);
when(indexColl.get(anyString())).thenReturn(index);
when(index.attach(isA(Args.class))).thenReturn(socket);
- when(socket.getOutputStream()).thenReturn(System.out);
Review Comment:
I think it was fine in this case. We cannot assume what the test was
expecting, so, let's revert this change.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/bind/PipeProvider.java:
##########
@@ -48,6 +50,7 @@
*/
public class PipeProvider extends ObjectReferenceBindingProvider {
+ private static final Logger LOG =
LoggerFactory.getLogger(PipeProvider.class);
Review Comment:
Revert in this class. It's used directly by CLI.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/Printer.java:
##########
@@ -52,25 +54,29 @@ default void printErr(Exception e) {
}
/**
- * Default printer uses System out print stream.
Review Comment:
Revert in this class. It's used directly by CLI.
##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesHelper.java:
##########
@@ -45,13 +45,16 @@
import org.apache.camel.dsl.jbang.core.common.YamlHelper;
import org.apache.camel.util.FileUtil;
import org.apache.camel.util.StringHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.Yaml;
/**
* Helper class provides access to cached Kubernetes client. Also provides
access to generic Json and Yaml mappers.
*/
public final class KubernetesHelper {
+ private static final Logger LOG =
LoggerFactory.getLogger(KubernetesHelper.class);
Review Comment:
Revert in this class. It's used directly by CLI.
##########
dsl/camel-kamelet-main/src/main/java/org/apache/camel/main/KameletMain.java:
##########
@@ -112,12 +112,15 @@
import org.apache.camel.support.service.ServiceHelper;
import org.apache.camel.support.startup.BacklogStartupStepRecorder;
import org.apache.camel.tooling.maven.MavenGav;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* A Main class for booting up Camel with Kamelet in standalone mode.
*/
public class KameletMain extends MainCommandLineSupport {
+ private static final Logger LOG =
LoggerFactory.getLogger(KameletMain.class);
Review Comment:
Revert in this class. It's used directly by CLI.
##########
tooling/openapi-rest-dsl-generator/src/main/java/org/apache/camel/generator/openapi/RestDslYamlGenerator.java:
##########
@@ -173,7 +173,11 @@ public String generate(final CamelContext context, boolean
generateRoutes) throw
}
}
- ObjectMapper mapper = new ObjectMapper(new
YAMLFactory().disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER));
Review Comment:
Revert this change. It enables a couple of features which may not be
required.
##########
components/camel-splunk/src/test/java/org/apache/camel/component/splunk/RawProducerTest.java:
##########
@@ -70,7 +73,7 @@ public void setup() throws IOException {
when(inputCollection.get(anyString())).thenReturn(input);
when(indexColl.get(anyString())).thenReturn(index);
when(index.attach(isA(Args.class))).thenReturn(socket);
- when(socket.getOutputStream()).thenReturn(System.out);
+ when(socket.getOutputStream()).thenReturn(baos);
Review Comment:
Ditto.
##########
catalog/camel-report-maven-plugin/src/main/java/org/apache/camel/maven/htmlxlsx/process/CoverageResultsProcessor.java:
##########
@@ -224,10 +228,9 @@ protected void gatherBestRouteCoverages() {
}
} catch (Exception t) {
// this is an edge case that needs to be identified. Log
some useful debugging information.
- System.out.println(t.getClass().toString());
- System.out.printf("routeID: %s%n", routeId);
- System.out.printf("route: %s%n", route);
- System.out.printf("mappedRoute: %s%n", mappedRoute != null
? mappedRoute.toString() : "null");
+ LOG.error("Error processing route coverage for routeId:
{}", routeId, t);
Review Comment:
Yeah, this one probably has to be reverted. I don't know how that output is
later processed, so, no worries to keep it with the default output stream. In
any case, provide a comment which clarify the intent that since this is a maven
tool, we prefer to maintain the older way.
--
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]
