gnodet-bot commented on code in PR #26792:
URL: https://github.com/apache/camel/pull/26792#discussion_r4092384324
##########
core/camel-core-xml/pom.xml:
##########
@@ -42,7 +42,19 @@
<artifactId>camel-core-engine</artifactId>
</dependency>
+ <!-- required logging api dependency -->
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
+
<!-- testing -->
+ <!-- test -->
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-core-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
Review Comment:
⚠️ **Duplicate dependency — `camel-core-engine` is declared twice.**
`camel-core-engine` already exists at compile scope (line 42, pre-existing).
This PR adds a second declaration at test scope (lines 52-57). Maven processes
the first declaration and silently ignores the second, so the test-scoped entry
is dead XML.
If the intent is to restrict this module to test usage only, the
pre-existing compile-scoped entry must be removed and replaced with this
test-scoped one. If compile scope is correct, remove this entire block.
Also: the `<!-- test -->` comment duplicates the existing `<!-- testing -->`
comment on line 51. One of them should be removed.
```suggestion
```
##########
core/camel-yaml-io/pom.xml:
##########
@@ -56,6 +51,18 @@
</dependency>
<!-- testing -->
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-core-model</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-util</artifactId>
+ </dependency>
Review Comment:
⚠️ **Production deps placed after `<!-- testing -->` comment — misleading
structure.**
`camel-api`, `camel-core-model`, and `camel-util` are compile-scope
(production) dependencies, but they are inserted immediately after the `<!--
testing -->` section header. A reader scanning this POM will assume everything
below that comment is test-scoped and may add `<scope>test</scope>` to these
deps in a future refactor, silently breaking the compile.
Move these three blocks to before the `<!-- testing -->` comment (i.e.,
after the `jackson-dataformat-yaml` dependency at line 51).
##########
tooling/openapi-rest-dsl-generator/pom.xml:
##########
@@ -94,6 +90,31 @@
</dependency>
<!-- test -->
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-core-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-api</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-support</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-util</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.camel</groupId>
+ <artifactId>camel-core-model</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ </dependency>
Review Comment:
⚠️ **Production deps placed after `<!-- test -->` comment — same issue as
`camel-yaml-io`.**
`camel-api`, `camel-support`, `camel-util`, `camel-core-model`, and
`slf4j-api` (lines 98-117) have no `<scope>test</scope>`, meaning they are
compile-scope (production) deps. Yet they are inserted immediately after `<!--
test -->` and before `junit-jupiter` and `assertj-core`, making them look like
test-only dependencies.
Move these five blocks above the `<!-- test -->` comment so the structure
accurately reflects their scope.
--
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]
