core,README: add docs about grpc-java-api-checker by jyane · Pull Request #4182 · grpc/grpc-java

jyane · 2018-03-07T15:18:56Z

No description provided.

ericgribkoff

Thanks for doing this! A couple comments below.

ericgribkoff · 2018-03-08T16:38:11Z

+
+Tools
+-----
+APIs that are annotated with `@ExperimentalApi` are subject to change, and APIs


I think we want to aim this more at library code that uses gRPC, as they may have less control of the ultimate gRPC version that gets used than an application developer directly using gRPC. Something like:

APIs that are annotated with @ExperimentalApi are subject to change, and APIs that are annotated with @Internal are for internal use by the gRPC library. Because APIs with these annotations could be modified or even removed in a future release, library code that other projects may depend on should not use these APIs. We recommend using the grpc-java-api-checker (an Error Prone plugin) to check for usages of @ExperimentalApi and @Internal in any library code that depends on gRPC.

I thought all users should not use @Internal APIs. I respect your opinion :)

That's fair. We have the wording in @Internal's javadoc about using these APIs to implement a custom load balancer or transport, but that's probably fine to leave out here:

APIs annotated with @Internal are for internal use by the gRPC library and should not be used by gRPC users. APIs annotated with @ExperimentalApi are subject to change in future releases, and library code that other projects may depend on should not use these APIs. We recommend using the grpc-java-api-checker (an Error Prone plugin) to check for usages of @ExperimentalApi and @Internal in any library code that depends on gRPC. It may also be used to check for @Internal usage or unintended @ExperimentalApi consumption in non-library code.

Currently quite a lot of our APIs are marked experimental, so I think the "you should not use @ExperimentalApi" statement in the original text is too strong. @ejona86 WDYT?

Oops, the load balancer reference is out-dated. Sent out #4192

Yeah, "not use" is too strong. For the library piece I think I'd say something closer to "Since libraries can not generally guarantee which version of gRPC is being used, most libraries should not use APIs annotated with @ExperimentalApi." But @ericgribkoff's most recent suggested text looks fine.

Thanks! I've fixed.

ericgribkoff · 2018-03-08T16:50:02Z

 * <p>Note: This annotation is intended only for gRPC library code. Users should not attach this
- * annotation to their own code.
+ * annotation to their own code. If you want to check APIs' usage, you can use
+ * grpc-java-api-checker.


How about:

* @see <a href="https://github.com/grpc/grpc/grpc-java-api-checker">grpc-java-api-checker</a>, an * Error Prone plugin to automatically check for usages of this API.

(Same for Internal.java)

jyane · 2018-03-10T02:52:07Z

@ericgribkoff PTAL

jyane · 2018-03-10T03:12:04Z

Wait... checkstyles are failed... I will fix it.

jyane · 2018-03-10T11:20:10Z

I've fixed but I've found new warnings on :javadoc. (d76746a)

.:grpc-core:javadocPicked up _JAVA_OPTIONS: -Xmx2048m -Xms512m
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/ExperimentalApi.java:52: warning - Tag @see: missing final '>': "<a href="https://github.com/grpc/grpc-java-api-checker">grpc-java-api-checker</a>, an Error
     Prone plugin to automatically check for usages of this API."
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/Internal.java:48: warning - Tag @see: missing final '>': "<a href="https://github.com/grpc/grpc-java-api-checker">grpc-java-api-checker</a>, an Error
     Prone plugin to automatically check for usages of this API."

2 warnings

and It could be reproduced on my local machine.

$ ./gradlew :grpc-core:javadoc

> Configure project :grpc-compiler
*** Building codegen requires Protobuf version 3.5.1-1
*** Please refer to https://github.com/grpc/grpc-java/blob/master/COMPILING.md#how-to-build-code-generation-plugin

> Task :grpc-core:javadoc
/Users/jyane/workspace/repos/github.com/jyane/grpc-java/core/src/main/java/io/grpc/ExperimentalApi.java:52: warning - Tag @see: missing final '>': "<a href="https://github.com/grpc/grpc-java-api-checker">grpc-java-api-checker</a>, an Error
     Prone plugin to automatically check for usages of this API."
/Users/jyane/workspace/repos/github.com/jyane/grpc-java/core/src/main/java/io/grpc/Internal.java:49: warning - Tag @see: missing final '>': "<a href="https://github.com/grpc/grpc-java-api-checker">grpc-java-api-checker</a>, an Error
      Prone plugin to automatically check for usages of this API."
2 warnings


BUILD SUCCESSFUL in 4s
6 actionable tasks: 1 executed, 5 up-to-date

Using a link with strings might not be permitted.
https://docs.oracle.com/javase/8/docs/technotes/tools/windows/javadoc.html#CHDDIEDI

Thus, I've removed @see. @ericgribkoff What do you think? I might be something wrong.

ericgribkoff

LGTM

jyane · 2018-03-28T01:22:09Z

I want to merge this PR once grpc-java-api-checker:1.1.0 was released.

ref: grpc/grpc-java-api-checker#15

jyane · 2018-03-29T03:58:00Z

Thanks!

core,README: add docs about grpc-java-api-checker

e544512

ejona86 requested a review from ericgribkoff March 8, 2018 15:50

ejona86 assigned ericgribkoff Mar 8, 2018

ericgribkoff reviewed Mar 8, 2018

View reviewed changes

jyane force-pushed the add-api-checker branch 2 times, most recently from 99acd0d to a8ff84c Compare March 10, 2018 02:50

address the comments

0b9ae12

jyane force-pushed the add-api-checker branch from a8ff84c to 0b9ae12 Compare March 10, 2018 02:51

ericgribkoff approved these changes Mar 10, 2018

View reviewed changes

jyane added 2 commits March 10, 2018 19:42

fix indent

d76746a

remove @see for javadoc warnings

24a3c4f

ericgribkoff approved these changes Mar 10, 2018

View reviewed changes

ejona86 approved these changes Mar 28, 2018

View reviewed changes

ericgribkoff merged commit 5af2515 into grpc:master Mar 28, 2018

jyane deleted the add-api-checker branch March 29, 2018 00:38

lock Bot locked as resolved and limited conversation to collaborators Jan 18, 2019

Conversation

jyane commented Mar 7, 2018

Uh oh!

ericgribkoff left a comment

Choose a reason for hiding this comment

Uh oh!

ericgribkoff Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

jyane Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

ericgribkoff Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

ejona86 Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

jyane Mar 10, 2018

Choose a reason for hiding this comment

Uh oh!

ericgribkoff Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

jyane Mar 10, 2018

Choose a reason for hiding this comment

Uh oh!

jyane commented Mar 10, 2018

Uh oh!

jyane commented Mar 10, 2018

Uh oh!

jyane commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericgribkoff left a comment

Choose a reason for hiding this comment

Uh oh!

jyane commented Mar 28, 2018

Uh oh!

jyane commented Mar 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jyane commented Mar 10, 2018 •

edited

Loading