chore: migrate to toml dependencies #56
Conversation
skjindal93
requested review from
tim-mwangi,
surajpuvvada and
avinashkolluru
as code owners
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## main #56 +/- ##
============================================
+ Coverage 69.20% 69.44% +0.24%
Complexity 106 106
============================================
Files 15 15
Lines 565 563 -2
Branches 33 32 -1
============================================
Hits 391 391
+ Misses 154 153 -1
+ Partials 20 19 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
gradle/libs.versions.toml
Outdated
| hypertrace-grpcUtils = "0.9.0" | ||
|
|
||
| guice = "5.1.0" | ||
| grpc = "1.48.0" |
There was a problem hiding this comment.
| grpc = "1.48.0" | |
| grpc = "1.50.0" |
(that's the version it'll get from grpcutils anyway)
gradle/libs.versions.toml
Outdated
|
|
||
| guice = "5.1.0" | ||
| grpc = "1.48.0" | ||
| jetty = "9.4.48.v20220622" |
There was a problem hiding this comment.
might as well bump this to latest too
| jetty = "9.4.48.v20220622" | |
| jetty = "9.4.49.v20220914" |
gradle/libs.versions.toml
Outdated
| typesafe-config = { module = "com.typesafe:config", version = "1.4.2" } | ||
| lombok = { module = "org.projectlombok:lombok", version = "1.18.22" } | ||
| slf4j-api = { module = "org.slf4j:slf4j-api", version = "1.7.32" } | ||
| apache-log4j-slf4jImpl = { module = "org.apache.logging.log4j:log4j-slf4j-impl", version = "2.17.1" } |
There was a problem hiding this comment.
| apache-log4j-slf4jImpl = { module = "org.apache.logging.log4j:log4j-slf4j-impl", version = "2.17.1" } | |
| apache-log4j-slf4jImpl = { module = "org.apache.logging.log4j:log4j-slf4j-impl", version = "2.19.0" } |
| @@ -6,17 +6,16 @@ plugins { | |||
| } | |||
|
|
|||
| dependencies { | |||
| api(project(":platform-service-framework")) | |||
| api(platform("io.grpc:grpc-bom:1.47.0")) | |||
There was a problem hiding this comment.
you missed the grpc-bom - that's key!
There was a problem hiding this comment.
I thought that was needed to sync grpc versions across (which is now achieved via toml), since other grpc libraries didn't really had versions mentioned.
There was a problem hiding this comment.
I probably should have explained and messaged this, I just started doing it inconsistently 😳
So the idea of the BOM is that by declaring it here, it should get inherited when this library is used as a dependency. The version catalog helps us declare versions consistently within a project, but the bom helps us resolve consistently across projects.
As an example of this, a common issue we see:
Imagine query service API was using grpc 1.50.0, and gateway service, running grpc 1.45.1 upgrades its query service api. it would inherit certain dependencies (grpc-stub, grpc-api etc.) at 1.50.0, but it would also have other grpc dependencies of its own (e.g. grpc-netty) at 1.45.1 leading to a version mismatch and runtime errors. By including the bom with our api's dependencies, we should (hopefully!) see grpc-netty upgraded to 1.50.0 instead. In this position, it actually works in the reverse direction - if an API were to bring in a lower version of a grpc lib that was not otherwise declared (less common scenario, since usually the servers have more grpc libs than the api packages), this would bring it up to the server's specified version.
|
thanks for doing this! |
This comment has been minimized.
This comment has been minimized.
Unit Test Results 9 files 9 suites 12s ⏱️ Results for commit 584ea7c. |
| testImplementation(libs.bundles.mockito) | ||
| testImplementation(libs.jetty.servlet) { | ||
| artifact { | ||
| classifier = "tests" |
There was a problem hiding this comment.
what is this classifier?
No description provided.