tinkerpop icon indicating copy to clipboard operation
tinkerpop copied to clipboard

[TINKERPOP-3060] Manage the versions of snappy-java and jackson-* centrally to avoid version conflict.

Open HappyHacker123 opened this issue 1 year ago • 4 comments

Currently some modules of Tinkerpop are using the same dependency. However, some of these dependencies' versions are not centrally managed and therefore cause discrepancy. This pr defines version in parent pom.xml centrally to avoid version conflict.

HappyHacker123 avatar Apr 08 '24 11:04 HappyHacker123

@xiazcy @Cole-Greer Could you please help me review this pr? Many thanks :)

HappyHacker123 avatar Apr 08 '24 11:04 HappyHacker123

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.49%. Comparing base (9b46b67) to head (8b1e8bd). Report is 54 commits behind head on 3.7-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##             3.7-dev    #2546      +/-   ##
=============================================
+ Coverage      76.14%   76.49%   +0.34%     
- Complexity     13152    13174      +22     
=============================================
  Files           1084     1059      -25     
  Lines          65160    61282    -3878     
  Branches        7285     7297      +12     
=============================================
- Hits           49616    46876    -2740     
+ Misses         12839    11892     -947     
+ Partials        2705     2514     -191     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 08 '24 12:04 codecov-commenter

Thanks for the PR @HappyHacker123! I like the improvement for the dependency version management. I do want to take a closer look into other pom files that might be able to take advantage of this. Note that we are in the middle of a release so we'll hold off on any merging and I'll revisit this PR once the release is done.

xiazcy avatar Apr 08 '24 18:04 xiazcy

Thanks for your response :) @xiazcy .

Note that we are in the middle of a release so we'll hold off on any merging and I'll revisit this PR once the release is done.

Sure, I'm fully supportive of holding off the pr until after the release is completed :).

I do want to take a closer look into other pom files that might be able to take advantage of this.

Actually i ran a check on the whole Tinkerpop project and found many other situation like this. If you need it, i'm happy to provide the whole list :).

HappyHacker123 avatar Apr 09 '24 13:04 HappyHacker123

Hi @HappyHacker123, much apologies for the late reply. I gave it some more thoughts, and while this will be a great improvement, I don't think we are quite ready to centralize these dependencies yet. The complexity is mainly due to the way dependencies are currently managed that’s leading to conflicts and may require module-specific versions, especially in hadoop and spark modules (as you may see some of the code comments in the poms). We are looking to improve this systemically, but before a decision is reached I think we’ll hold off on this PR and close it for now.

If you are still interested and have thoughts or recommendations related to dependency management, chime into our discussions for removing gremlin-shaded and JPMS. We'd love to see more opinions on these topics. Again, thank you for taking the time submitting this PR!

xiazcy avatar Jun 04 '24 18:06 xiazcy