featzhang commented on code in PR #28154:
URL: https://github.com/apache/flink/pull/28154#discussion_r3235973854


##########
flink-models/flink-model-triton/pom.xml:
##########
@@ -34,10 +34,21 @@ under the License.
 
        <properties>
                <okhttp.version>4.12.0</okhttp.version>
-               <jackson.version>2.15.2</jackson.version>
                <test.gson.version>2.11.0</test.gson.version>
        </properties>
 
+       <dependencyManagement>
+               <dependencies>
+                       <dependency>
+                               <groupId>com.fasterxml.jackson</groupId>
+                               <artifactId>jackson-bom</artifactId>
+                               <version>${jackson-bom.version}</version>

Review Comment:
   The root `pom.xml` already imports `jackson-bom` at `${jackson-bom.version}` 
inside the top-level `<dependencyManagement>` (lines 644-649 on master, added 
by FLINK-39580). Since `flink-models/pom.xml` declares `flink-parent` directly 
and adds no `<dependencyManagement>` of its own, every submodule already 
inherits the BOM-managed jackson versions. Re-importing the same BOM at the 
same version here is redundant and, more importantly, becomes a silent drift 
source the next time the root bumps `jackson-bom.version` and someone forgets 
to delete this block.
   
   For reference, the sibling `flink-model-openai/pom.xml` only declares a 
`<dependencyManagement>` for things the parent does *not* manage (kotlin 
stdlib), and lets jackson fall through.
   
   Suggest dropping the `<dependencyManagement>` block from this pom entirely — 
the rest of the diff (deleting `<jackson.version>` + the per-dep `<version>` 
overrides) is sufficient on its own to pick up the BOM-resolved versions, which 
is also what the NOTICE update reflects.



-- 
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]

Reply via email to