Copilot commented on code in PR #17836:
URL: https://github.com/apache/pinot/pull/17836#discussion_r2902645104
##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -59,6 +59,39 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.pinot</groupId>
+ <artifactId>pinot-kafka-3.0</artifactId>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka_${scala.compat.version}</artifactId>
+ <version>${kafka3.version}</version>
+ <classifier>test</classifier>
+ <scope>test</scope>
Review Comment:
These Kafka test dependencies are already version-managed in the root
`pom.xml` via `dependencyManagement` (including the `test` classifiers).
Consider removing the explicit `<version>${kafka3.version}</version>` here to
avoid redundant version declarations and reduce drift risk if the managed
version changes.
##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -59,6 +59,39 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.pinot</groupId>
+ <artifactId>pinot-kafka-3.0</artifactId>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka_${scala.compat.version}</artifactId>
+ <version>${kafka3.version}</version>
+ <classifier>test</classifier>
+ <scope>test</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>commons-logging</groupId>
+ <artifactId>commons-logging</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka-clients</artifactId>
+ <version>${kafka3.version}</version>
+ <classifier>test</classifier>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka-server-common</artifactId>
+ <version>${kafka3.version}</version>
Review Comment:
Same as above for `kafka-server-common`: since it’s already version-managed
in the root `dependencyManagement`, consider removing the explicit
`<version>${kafka3.version}</version>` here to avoid duplicate version
declarations.
```suggestion
```
##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -59,6 +59,39 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.pinot</groupId>
+ <artifactId>pinot-kafka-3.0</artifactId>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka_${scala.compat.version}</artifactId>
+ <version>${kafka3.version}</version>
+ <classifier>test</classifier>
+ <scope>test</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>commons-logging</groupId>
+ <artifactId>commons-logging</artifactId>
+ </exclusion>
+ </exclusions>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.kafka</groupId>
+ <artifactId>kafka-clients</artifactId>
+ <version>${kafka3.version}</version>
Review Comment:
`kafka-clients` (including the `test` classifier) appears to be
version-managed in the root `dependencyManagement`. To keep this module POM
consistent with the other dependencies here, consider dropping the explicit
`<version>${kafka3.version}</version>` and relying on the managed version
instead.
```suggestion
```
--
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]
