Conversation
varkey98
requested review from
arun-dhingra,
bhubam,
harshit-kumar-v2 and
souravd86
varkey98
changed the title
| } | ||
|
|
||
| configure<DependencyPluginSettingExtension> { | ||
| catalogVersion.set("0.3.51") |
There was a problem hiding this comment.
this strangely doesnt have any lock file updates
gradle/libs.versions.toml
Outdated
| [versions] | ||
| hypertrace-grpcutils = "0.13.14" | ||
| jetty = "11.0.24" | ||
| jetty = "12.1.6" |
There was a problem hiding this comment.
We cannot move to jetty 12+ until we have not moved all over services to java 17+ . I think there are services which are running on java 11.
There was a problem hiding this comment.
This is an older commit, pls get the latest changes
gradle/libs.versions.toml
Outdated
| [versions] | ||
| hypertrace-grpcutils = "0.13.14" | ||
| jetty = "11.0.24" | ||
| jetty = "11.0.26" |
There was a problem hiding this comment.
Discussed offline. Would prefer getting rid of this explicit dependency here and using the one from commonLibs which means you need to upgrade the version in hypertrace-bom first - https://github.com/hypertrace/hypertrace-bom/blob/846e833212c27988a3b0118e6ae88f5124aed200/gradle/libs.versions.toml#L12
There was a problem hiding this comment.
I've moved to using guice and jetty from the bom. Should I move the other local deps as well to bom?
There was a problem hiding this comment.
Whatever is already in the bom, you should use it. The ones that aren't can remain in this repo repo.
There was a problem hiding this comment.
But, you'd probably need to first update the version in the hypertrace-bom, publish that and then upgrade the locks too here in this repo for the versions to be picked up.
There was a problem hiding this comment.
the version for jetty atleast is already with vuln fix in hypertrace bom
There was a problem hiding this comment.
Also do we know why we refer both bom and local version for grpc in here?
https://github.com/hypertrace/service-framework/blob/main/platform-grpc-service-framework/build.gradle.kts#L10-L11
There was a problem hiding this comment.
This should be coming from bom only. Please refer the one in bom and remove any explicit references in the repo.
There was a problem hiding this comment.
Ignore. I see that you already fixed it.
3 participants
No description provided.