Skip to content

chore: migrate to toml dependencies - #56

Open
skjindal93 wants to merge 5 commits into
mainfrom
toml
Open

chore: migrate to toml dependencies#56
skjindal93 wants to merge 5 commits into
mainfrom
toml

Conversation

@skjindal93

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@codecov

codecov Bot commented Oct 21, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.44%. Comparing base (f288817) to head (cfbb4dc).
⚠️ Report is 82 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main      #56   +/-   ##
=========================================
  Coverage     69.44%   69.44%           
  Complexity      106      106           
=========================================
  Files            15       15           
  Lines           563      563           
  Branches         32       32           
=========================================
  Hits            391      391           
  Misses          153      153           
  Partials         19       19           
Flag Coverage Δ
unit 69.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread gradle/libs.versions.toml Outdated
hypertrace-grpcUtils = "0.9.0"

guice = "5.1.0"
grpc = "1.48.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
grpc = "1.48.0"
grpc = "1.50.0"

(that's the version it'll get from grpcutils anyway)

Comment thread gradle/libs.versions.toml Outdated

guice = "5.1.0"
grpc = "1.48.0"
jetty = "9.4.48.v20220622"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might as well bump this to latest too

Suggested change
jetty = "9.4.48.v20220622"
jetty = "9.4.49.v20220914"

Comment thread 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" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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" }


dependencies {
api(project(":platform-service-framework"))
api(platform("io.grpc:grpc-bom:1.47.0"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you missed the grpc-bom - that's key!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@aaron-steinfeld

Copy link
Copy Markdown
Contributor

thanks for doing this!

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

Unit Test Results

  9 files    9 suites   12s ⏱️
31 tests 31 ✔️ 0 💤 0 ❌

Results for commit 584ea7c.

testImplementation(libs.bundles.mockito)
testImplementation(libs.jetty.servlet) {
artifact {
classifier = "tests"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this classifier?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants