HBASE-28883 Manage hbase-thirdparty transitive dependencies via BOM pom #124
Conversation
|
Also, please see code change required in HBase to consume this: NihalJain/hbase@0b51c71 NOTE: Could not remove the hardcoding of internal.protobuf.version as with BOM seems its not possible to pass on properties. Have kept relevant code still here in this PR. Will cleanup based on review. CC: @ndimiduk |
| </dependency> | ||
| <dependency> | ||
| <groupId>io.netty</groupId> | ||
| <artifactId>netty-bom</artifactId> |
There was a problem hiding this comment.
@Apache9 mentioned in https://github.com/apache/hbase/pull/6295/files#r1775002522, we may not need to bump netty4 as it is for zk/hadoop. Same for error prone.
We can decide what all dependencies we would want to keep here. Here's a exhaustive list of dependencies i found relevant.
This comment has been minimized.
This comment has been minimized.
NihalJain
force-pushed
the
HBASE-28883
branch
from
546de8c to
0f3dae4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
| <plugins> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>flatten-maven-plugin</artifactId> |
There was a problem hiding this comment.
Why is the flatten plugin used for the bom? By definition, there should be no dependencies via this pom.
There was a problem hiding this comment.
Hi @ndimiduk To preserve the dependencyManagement section in the bom, I had to choose flatten mode: bom. We have flattenMode=oss in parent but for the bom we need to have flattenMode=bom. Hence, I had to add this in order to override the flatten mode defined in parent.
Please see https://www.mojohaus.org/flatten-maven-plugin/apidocs/org/codehaus/mojo/flatten/FlattenMode.html
| </dependency> | ||
| <dependency> | ||
| <groupId>javax.servlet</groupId> | ||
| <artifactId>javax.servlet-api</artifactId> |
There was a problem hiding this comment.
Why is this one included? There's no mention of it in the hbase.git/pom
There was a problem hiding this comment.
We do have this at https://github.com/apache/hbase/blob/master/pom.xml#L1555
Although I am not sure if we need to keep this in sync.
|
🎊 +1 overall
This message was automatically generated. |
|
Actually should we have logging dependencies here? |
Hi @ndimiduk Please refer apache/hbase#6366
Yes we can technically have all dependencies we want to keep common across our projects. Please let me know if we would like to add log4j and other logging jars over here. Also please let me know if we should evaluate other libs which can be moved to this bom, for eg: junit, mockito, hamcrest etc? |
| <dependencyManagement> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.google.protobuf</groupId> |
There was a problem hiding this comment.
we should not have this as per observation in apache/hbase#6366 (comment)
CC: @ndimiduk
No description provided.