-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
all: migrate gradle build to java-library plugin #7001
Conversation
65b87d5
to
115825e
Compare
115825e
to
582b48c
Compare
@@ -10,19 +10,20 @@ plugins { | |||
description = 'gRPC: Protobuf' | |||
|
|||
dependencies { | |||
compile project(':grpc-api'), | |||
api project(':grpc-api'), | |||
libraries.jsr305, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why expose this one?
libraries.animalsniffer_annotations | ||
guavaDependency 'compile' | ||
api project(':grpc-context'), | ||
libraries.jsr305 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why expose this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is part of public API signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean annotations like @javax.annotation.concurrent.ThreadSafe
, @javax.annotation.Nullable
are part of some public API signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Much easier to review.
} | ||
compile libraries.netty_tcnative | ||
implementation libraries.protobuf | ||
implementation libraries.truth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this dep added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to build without it:
/Users/zdapeng/git/grpc-java/gae-interop-testing/gae-jdk8/src/main/java/io/grpc/testing/integration/NettyClientInteropServlet.java:137: error: cannot find symbol
assertTrue(builder instanceof NettyChannelBuilder);
The reason should be that truth
was a dep of grpc-interop-testing
which is now changed from compile to implementation. Only api/compile dep is transitive for compileClasspath across subproject. Implementation dep is transitive for runtimeClasspath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertTrue
is a junit method. This should probably be implementation libraries.junit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. My mistake. Done.
@@ -31,7 +31,7 @@ for (subproject in rootProject.subprojects) { | |||
} | |||
|
|||
dependencies { | |||
compile subprojects.minus(project(':grpc-protobuf-lite')) | |||
implementation subprojects.minus(project(':grpc-protobuf-lite')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we should use api
here. @ejona86 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is probably better to use api
to avoid breaking people. That said, I don't have too much sympathy for someone using grpc-all these days (vs early-on-pre-1.0 when it was used more heavily).
api
for dependencies that are part of grpc public api signatures.compile
,testCompile
,runtime
andtestRuntime
.compile
/implementation
dependency for our project. Ideally we should include both X and Y explicitly asimplementation
dependency for our project, but in this PR we don't add the missing Y if it is previously missing. A followup PR will try to add all such missing dependencies by setting