Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly declare transitive dependencies that we use directly #3164

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

Dependency-analysis plugin detected that some dependencies that we use directly not declared in our build configuration.

Modifications:

  1. This PR only adds missing dependencies. It does not remove anything. If you see a removed line, it only means I sorted existing dependencies in alphabetical order.
  2. Adds opentelemetry-instrumentation-bom to servicetalk-dependencies.

Result:

Addresses "These transitive dependencies should be declared directly" warning from dependency-analysis plugin.

Risk:

Because we only add missed dependencies, there is no risk of breaking existing users.

Motivation:

Dependency-analysis plugin detected that some dependencies that we use
directly not declared in our build configuration.

Modifications:

1. This PR only adds missing dependencies. It does not remove anything.
If you see a removed line, it only means I sorted existing dependencies
in alphabetical order.
2. Adds `opentelemetry-instrumentation-bom` to
`servicetalk-dependencies`.

Result:

Addresses "These transitive dependencies should be declared directly"
warning from dependency-analysis plugin.
@idelpivnitskiy idelpivnitskiy requested a review from daschl January 10, 2025 17:16
@idelpivnitskiy idelpivnitskiy self-assigned this Jan 10, 2025
@idelpivnitskiy idelpivnitskiy merged commit 373299a into apple:main Jan 10, 2025
11 checks passed
@idelpivnitskiy idelpivnitskiy deleted the add-transitive-deps branch January 10, 2025 19:38
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Jan 11, 2025
Motivation:

Dependency-analysis plugin detected that some `api` dependencies are
unused and can be either removed or downgraded to `implementation` /
`testImplementation` scopes.

Modifications:

- Remove unused `api` dependencies.
- Downgrade some to `implementation` / `testImplementation` scopes.
- Regenerate lock files.

Result:

No more warnings from dependency-analysis plugin.

Risk for users:

Minimal. They can be broken only if they use some dependency at compile
time without explicitly declaring it in their build, which is their
mistake.

Highlights:
- Most changes are related to `servicetalk-*` dependencies,
users likely have them anyway via other modules.
- It's safe to remove `opentelemetry-api` from
`opentelemetry-asynccontext` module because in apple#3164 we added
`opentelemetry-context`.
- Removal of `proto-google-common-protos` should not be an issue because
it's already an `api` dependency for `grpc-api` module.
idelpivnitskiy added a commit that referenced this pull request Jan 13, 2025
Motivation:

Dependency-analysis plugin detected that some `api` dependencies are
unused and can be either removed or downgraded to `implementation` /
`testImplementation` scopes.

Modifications:

- Remove unused `api` dependencies.
- Downgrade some to `implementation` / `testImplementation` scopes.
- Regenerate lock files.

Result:

No more warnings from dependency-analysis plugin.

Risk for users:

Minimal. They can be broken only if they use some dependency at compile
time without explicitly declaring it in their build, which is their
mistake.

Highlights:
- Most changes are related to `servicetalk-*` dependencies,
users likely have them anyway via other modules.
- It's safe to remove `opentelemetry-api` from
`opentelemetry-asynccontext` module because in #3164 we added
`opentelemetry-context`.
- Removal of `proto-google-common-protos` should not be an issue because
it's already an `api` dependency for `grpc-api` module.
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.

2 participants