Skip to content

Conversation

@creamsoup
Copy link
Contributor

adapted from gax-java's BOM: googleapis/gax-java#607

some of examples with maven pom is also converted to use bom.

resolves #3954

@creamsoup creamsoup requested a review from ejona86 January 3, 2019 23:08
@creamsoup
Copy link
Contributor Author

creamsoup commented Jan 3, 2019

can you take a close look at the scope of dependency, and also list of dependencies in BOM?
I followed dropwizard like bom which includes all dependencies already in the build.gradle and all the modules.
for gradle projects, the dependencies of each module may not be needed? With gradle5 officially support BOM, those can be useful if we consume this in the gradle build file.

do we want to upgrade gradle 5 and use BOM in /build.gradle? or we remove non io.grpc dependencies from BOM, so we don't need to maintain two versions if we want to keep current version of gradle.

Copy link
Contributor

@dapengzhang0 dapengzhang0 left a comment

Choose a reason for hiding this comment

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

In general LGTM. The bom/build.gradle is more complicated than I expected though.

@dapengzhang0
Copy link
Contributor

dapengzhang0 commented Jan 10, 2019

Is <artifactId>maven-enforcer-plugin</artifactId> in examples/pom.xml needed anymore when BOM is used?

Copy link

@TheIndifferent TheIndifferent left a comment

Choose a reason for hiding this comment

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

Please do not ping transitive dependencies via BOM, that should be done differently.

@ejona86
Copy link
Member

ejona86 commented Jan 15, 2019

@dapengzhang0, maven-enforcer-plugin is still necessary after this to ensure the BOM is not accidentally downgrading a version, there may be mulitple BOMs, and because all Maven users should really be using requireUpperBoundDeps for their own sanity.

@ejona86 ejona86 self-assigned this Jan 16, 2019
@creamsoup
Copy link
Contributor Author

Quite a few major changes are made. PTAL
sample pom: https://paste.googleplex.com/6567224605147136

@creamsoup creamsoup merged commit ef6b616 into grpc:master Feb 1, 2019
@creamsoup creamsoup deleted the bom branch February 1, 2019 22:00
@lock lock bot locked as resolved and limited conversation to collaborators May 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create BOM for grpc-java

7 participants