Upgrade error_prone_annotations to 2.3.4 #6574

suztomo · 2019-12-27T18:57:31Z

Upgrading error_prone_annotations dependency from 2.3.3 to 2.3.4.

Motivation

Guava 28.2 has error_prone_annotations 2.3.4. An upper-bound-check detected gRPC has 2.3.3:

googleapis/java-websecurityscanner#32 (comment)

suztomo · 2019-12-27T18:58:24Z

repositories.bzl

        server_urls = ["https://repo.maven.apache.org/maven2/"],
-        artifact_sha256 = "ec59f1b702d9afc09e8c3929f5c42777dec623a6ea2731ac694332c7d7680f5a",
+        artifact_sha256 = "baf7d6ea97ce606c53e11b6854ba5f2ce7ef5c24dddf0afa18d1260bd25b002c",


suztomo@suxtomo24:~/grpc-java$ shasum -a 256 ~/Downloads/error_prone_annotations-2.3.4.jar baf7d6ea97ce606c53e11b6854ba5f2ce7ef5c24dddf0afa18d1260bd25b002c /usr/local/google/home/suztomo/Downloads/error_prone_annotations-2.3.4.jar

suztomo · 2019-12-27T19:06:02Z

Investigating the error.

> Task :grpc-alts:extractIncludeProto FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':grpc-alts:extractIncludeProto'.
> Could not resolve all dependencies for configuration ':grpc-alts:compile'.
   > Conflict(s) found for the following module(s):
       - com.google.errorprone:error_prone_annotations between versions 2.3.4 and 2.3.3
     Run with:
         --scan or
         :grpc-alts:dependencyInsight --configuration compile --dependency com.google.errorprone:error_prone_annotations
     to get more insight on how to solve the conflict.

creamsoup · 2019-12-27T19:13:51Z

there are few more places to update versions

android/build.gradle
build.gradle:266 this will fix the error
cronet/build.gradle

also, looks like 2.3.4 has some improvement in GuardedBy and UnnecessaryAnonymousClass. you may needs to fix this as well. I believe the GuardedBy should be fixed by #6566

suztomo · 2019-12-27T19:30:51Z

@creamsoup Thank you for the guide. I'll wait for #6566 then.

suztomo@suxtomo24:~/grpc-java$ ./gradlew build -DskipCodegen=true
*** Skipping the build of codegen and compilation of proto files because skipCodegen=true

> Task :grpc-core:compileJava
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/RetriableStream.java:141: error: [GuardedBy] This access should be guarded by 'this.scheduledRetry.lock'; instead found: 'this.lock'
        retryFuture = scheduledRetry.markCancelled();
                                    ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/RetriableStream.java:149: error: [GuardedBy] This access should be guarded by 'this.scheduledHedging.lock'; instead found: 'this.lock'
        hedgingFuture = scheduledHedging.markCancelled();
                                        ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/RetriableStream.java:359: error: [GuardedBy] This access should be guarded by 'this.scheduledHedging.lock'; instead found: 'this.lock'
      futureToBeCancelled = scheduledHedging.markCancelled();
                                            ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/RetriableStream.java:395: error: [GuardedBy] This access should be guarded by 'HedgingRunnable.this.scheduledHedgingRef.lock'; instead found: 'RetriableStream.this.lock'
                if (scheduledHedgingRef.isCancelled()) {
                                       ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/RetriableStream.java:702: error: [GuardedBy] This access should be guarded by 'this.scheduledHedging.lock'; instead found: 'this.lock'
        futureToBeCancelled = scheduledHedging.markCancelled();
                                              ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:125: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final AuthenticationProvider DEFAULT_AUTHENTICATOR = new AuthenticationProvider() {
                                              ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  PasswordAuthentication defaultAuthenticator('?
/usr/local/google/home/suztomo/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:143: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final Supplier<ProxySelector> DEFAULT_PROXY_SELECTOR =
                                               ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  ProxySelector defaultProxySelector() {'?
5 errors
2 warnings

suztomo · 2019-12-27T19:37:02Z

core/build.gradle

+            libraries.errorprone // to avoid using perfmark's dependency
+    compile (libraries.perfmark) {
+        exclude group: 'com.google.errorprone', module: 'error_prone_annotations'
+    }


Gradle complained perfmark's errorprone annotation 2.3.3.

https://travis-ci.org/grpc/grpc-java/jobs/630091143?utm_medium=notification&utm_source=github_status

Yes. You did the appropriate thing here, although we normally put a comment saying we are using version X instead of version Y (so that in the future we can tell if the exclude is old and should be removed).

Our build enforces dependency convergence because I've not found a better approach to detecting requireUpperBoundDeps violations from within Gradle (because they aren't a problem in Gradle...).

Updated comment as "prefer our version to perfmark's 2.3.3".

elharo

I don't think this should be done in isolation. It requires an upgrade to guava and protobuf at the same time, and that means we need a protobuf release that updates its guava and errorprone first.

…ions

suztomo · 2019-12-30T18:54:58Z

@elharo I see you already raised a PR for protobuf-java on Guava upgrade. Thanks.

ejona86 · 2019-12-30T18:55:00Z

@elharo, why do you say this "requires an upgrade to guava and protobuf at the same time"?

suztomo · 2019-12-30T18:56:33Z

Still build failure due to GuardedBy. I'll wait for #6578.

> Task :grpc-okhttp:compileTestJava FAILED
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java:281: warning: [SameNameButDifferent] The name `Handler` refers to [java.util.logging.Handler, io.grpc.okhttp.internal.framed.FrameReader.Handler] within this file. It may be confusing to have the same name refer to multiple types. Consider qualifying them for clarity.
    Handler handler = new Handler() {
    ^
    (see https://errorprone.info/bugpattern/SameNameButDifferent)
  Did you mean 'public boolean nextFrame(FrameReader.Handler handler) throws IOException {'?
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java:281: warning: [SameNameButDifferent] The name `Handler` refers to [java.util.logging.Handler, io.grpc.okhttp.internal.framed.FrameReader.Handler] within this file. It may be confusing to have the same name refer to multiple types. Consider qualifying them for clarity.
    Handler handler = new Handler() {
                          ^
    (see https://errorprone.info/bugpattern/SameNameButDifferent)
  Did you mean 'public boolean nextFrame(FrameReader.Handler handler) throws IOException {'?
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java:281: warning: [SameNameButDifferent] The name `Handler` refers to [java.util.logging.Handler, io.grpc.okhttp.internal.framed.FrameReader.Handler] within this file. It may be confusing to have the same name refer to multiple types. Consider qualifying them for clarity.
    Handler handler = new Handler() {
                          ^
    (see https://errorprone.info/bugpattern/SameNameButDifferent)
  Did you mean 'public boolean nextFrame(FrameReader.Handler handler) throws IOException {'?
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java:2130: warning: [SameNameButDifferent] The name `Handler` refers to [java.util.logging.Handler, io.grpc.okhttp.internal.framed.FrameReader.Handler] within this file. It may be confusing to have the same name refer to multiple types. Consider qualifying them for clarity.
    public boolean nextFrame(Handler handler) throws IOException {
                             ^
    (see https://errorprone.info/bugpattern/SameNameButDifferent)
  Did you mean 'public boolean nextFrame(FrameReader.Handler handler) throws IOException {'?
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:132: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(1234);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:152: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(1234);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:165: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(3);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:181: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(3);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:205: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(3);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:238: error: [GuardedBy] This access should be guarded by 'lock', which could not be resolved
    verify(transport, times(0)).streamReadyToStart(isA(OkHttpClientStream.class));
                               ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:243: error: [GuardedBy] This access should be guarded by 'lock', which could not be resolved
    verify(transport).streamReadyToStart(eq(stream));
                     ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/usr/local/google/home/suztomo/grpc-java/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java:244: error: [GuardedBy] This access should be guarded by 'this.stream.transportState().lock', which is not currently held
    stream.transportState().start(3);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
8 errors
4 warnings

FAILURE: Build failed with an exception.

ejona86 · 2019-12-30T18:57:02Z

It looks like my #6553 will get in before this one. If so, then this will need to be updated to change IO_GRPC_GRPC_JAVA_ARTIFACTS of repositories.bzl as well. Having yet another place to change is annoying in the short-term, but pretty soon we'll delete all the manual jvm_maven_import_external calls.

elharo · 2019-12-30T19:00:47Z

Because Guava and protobuf also pull in errorprone_annotations, and we'd like them to be in sync as much as possible. I don't think there's anything we actually need in error_prone_annotations 2.3.4. The reason to do this update in the first place is to bring everything into sync.

ejona86 · 2019-12-30T19:09:47Z

Upgrading everything in lockstep is silly and not really productive. Bumping versions when new ones come out is fine, but saying we need to bump guava and protobuf at the same time just adds risk and doesn't help anyone. If we happen to bump all of the at the same time, that's fine. But grpc-java should not be party to any goals of dependency convergence.

…ions

suztomo · 2019-12-30T20:34:06Z

IO_GRPC_GRPC_JAVA_ARTIFACTS of repositories.bzl as well.

Done.

Travis complains: https://travis-ci.org/grpc/grpc-java/jobs/631065748?utm_medium=notification&utm_source=github_status

> Task :grpc-core:compileJava
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:125: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final AuthenticationProvider DEFAULT_AUTHENTICATOR = new AuthenticationProvider() {
                                              ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  PasswordAuthentication defaultAuthenticator('?
/home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:143: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.
  private static final Supplier<ProxySelector> DEFAULT_PROXY_SELECTOR =
                                               ^
    (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass)
  Did you mean 'private static  ProxySelector defaultProxySelector() {'?
error: warnings found and -Werror specified
1 error
2 warnings

suztomo · 2019-12-30T20:37:16Z

core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java

@@ -122,6 +122,8 @@
  // $ sudo tail -f /var/log/squid/access.log

  private static final Logger log = Logger.getLogger(ProxyDetectorImpl.class.getName());
+
+  @SuppressWarnings("UnnecessaryAnonymousClass") // grpc-java targets Java 7 (no method references)


Travis CI complained: https://travis-ci.org/grpc/grpc-java/jobs/631065748?utm_medium=notification&utm_source=github_status

> Task :grpc-core:compileJava /home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:125: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead. private static final AuthenticationProvider DEFAULT_AUTHENTICATOR = new AuthenticationProvider() { ^ (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass) Did you mean 'private static PasswordAuthentication defaultAuthenticator('? /home/travis/build/grpc/grpc-java/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java:143: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead. private static final Supplier<ProxySelector> DEFAULT_PROXY_SELECTOR = ^ (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass) Did you mean 'private static ProxySelector defaultProxySelector() {'? error: warnings found and -Werror specified 1 error 2 warnings

Suppressing this looks appropriate. There's a reasonable number of these. In the future if there are lots of a particular failure that doesn't make sense like this you can disable the check altogether. See what we've done for JdkObsolete in the main build.gradle for an example of that. In this case it's on the border of silencing them globally or in each file (so what you've done is fine).

Thank you for the guidance. I choose to suppress UnnecessaryAnonymousClass at build.gradle because the decision of the suppression is project level (choosing Java 7 compatible source; no method references).

suztomo

Travis CI passes. Would you merge this?

suztomo · 2019-12-31T04:39:24Z

core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java

@@ -122,6 +122,8 @@
  // $ sudo tail -f /var/log/squid/access.log

  private static final Logger log = Logger.getLogger(ProxyDetectorImpl.class.getName());
+
+  @SuppressWarnings("UnnecessaryAnonymousClass") // grpc-java targets Java 7 (no method references)


Thank you for the guidance. I choose to suppress UnnecessaryAnonymousClass at build.gradle because the decision of the suppression is project level (choosing Java 7 compatible source; no method references).

suztomo · 2019-12-31T04:43:47Z

okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Http2.java

@@ -586,10 +587,12 @@ void frameHeader(int streamId, int length, byte type, byte flags) throws IOExcep
    }
  }

+  @FormatMethod


The build failed by https://errorprone.info/bugpattern/AnnotateFormatMethod

suztomo · 2019-12-31T05:48:05Z

context/src/main/java/io/grpc/Deadline.java

@@ -169,6 +169,7 @@ public Deadline offset(long offset, TimeUnit units) {
   * {@link TimeUnit#convert}. If there is no time remaining, the returned duration is how
   * long ago the deadline expired.
   */
+  @SuppressWarnings("PreferJavaTimeOverload") // Duration class not available in Java 7


The build for Java 11 failed without this suppresswarnings.

> Task :grpc-context:compileJava /home/travis/build/grpc/grpc-java/context/src/main/java/io/grpc/Deadline.java:177: warning: [PreferJavaTimeOverload] Prefer using java.time-based APIs when available. Note that this checker does not and cannot guarantee that the overloads have equivalent semantics, but that is generally the case with overloaded methods. return unit.convert(deadlineNanos - nowNanos, TimeUnit.NANOSECONDS); ^ (see https://errorprone.info/bugpattern/PreferJavaTimeOverload) Did you mean 'return unit.convert(Duration.ofNanos(deadlineNanos - nowNanos));'? error: warnings found and -Werror specified 1 error 1 warning

https://travis-ci.org/grpc/grpc-java/jobs/631194184?utm_medium=notification&utm_source=github_status

suztomo · 2019-12-31T05:51:43Z

core/build.gradle

+plugins.withId("java") {
+    compileJmhJava {
+        // This project targets Java 7 (no method references)
+        options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF)
+    }
+}


grpc-core has special task compileJmhJava. Without this configuration, the build failed:

> Task :grpc-core:compileJmhJava FAILED /Users/suztomo/grpc-java/core/src/jmh/java/io/grpc/internal/SerializingExecutorBenchmark.java:58: warning: [UnnecessaryAnonymousClass] Implementing a functional interface is unnecessary; prefer to implement the functional interface method directly and use a method reference instead. private final Runnable phaserRunnable = new Runnable() { ^ (see https://errorprone.info/bugpattern/UnnecessaryAnonymousClass) Did you mean 'private void phaserRunnable() {'? error: warnings found and -Werror specified 1 error 1 warning

suztomo · 2019-12-31T05:52:20Z

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -603,7 +603,7 @@ public void callsAndShutdownAndShutdownNow() {
  }

  private void subtestCallsAndShutdown(boolean shutdownNow, boolean shutdownNowAfterShutdown) {
-    FakeNameResolverFactory nameResolverFactory =
+    ManagedChannelImplTest.FakeNameResolverFactory nameResolverFactory =


SameNameButDifferent.

suztomo · 2019-12-31T06:02:01Z

build.gradle

+            // This project targets Java 7 (no method references)
+            options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF)
+            // This project targets Java 7 (no time.Duration class)
+            options.errorprone.check("PreferJavaTimeOverload", CheckSeverity.OFF)


Without PreferJavaTimeOverload suppression, the build for Java 11 fails:

> Task :grpc-context:compileJava FAILED /Users/suztomo/grpc-java/context/src/main/java/io/grpc/Deadline.java:177: warning: [PreferJavaTimeOverload] Prefer using java.time-based APIs when available. Note that this checker does not and cannot guarantee that the overloads have equivalent semantics, but that is generally the case with overloaded methods. return unit.convert(deadlineNanos - nowNanos, TimeUnit.NANOSECONDS); ^ (see https://errorprone.info/bugpattern/PreferJavaTimeOverload) Did you mean 'return unit.convert(Duration.ofNanos(deadlineNanos - nowNanos));'? error: warnings found and -Werror specified 1 error 1 warning

Setting this suppression at project level (rather than annotation) because whole grpc-java project targets Java 7, which does not have java.time.Duration.

suztomo · 2019-12-31T06:34:42Z

okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java

@@ -127,6 +127,7 @@ public void closed(
  }

  @Test
+  @SuppressWarnings("GuardedBy")


Without this suppression, the build fails.
https://travis-ci.org/grpc/grpc-java/jobs/631198698?utm_medium=notification&utm_source=github_status

#6578 will take care of the root cause.

suztomo · 2019-12-31T06:36:20Z

services/src/test/java/io/grpc/services/BinaryLogProviderTest.java

@@ -127,7 +126,7 @@ public void wrapChannel_handler() throws Exception {
          MethodDescriptor<RequestT, ResponseT> methodDescriptor, CallOptions callOptions) {
        return new NoopClientCall<RequestT, ResponseT>() {
          @Override
-          public void start(Listener<ResponseT> responseListener, Metadata headers) {
+          public void start(ClientCall.Listener<ResponseT> responseListener, Metadata headers) {


SameNameButDifferent

suztomo · 2019-12-31T12:18:42Z

build.gradle

@@ -279,9 +279,17 @@ subprojects {
            }
        }

+        compileJava {
+            // This project targets Java 7 (no method references)
+            options.errorprone.check("UnnecessaryAnonymousClass", CheckSeverity.OFF)


Suppressing UnnecessaryAnonymousClass as project-level suppression, because grpc-java targets Java 7 source.

suztomo · 2019-12-31T12:21:32Z

core/build.gradle

+            libraries.errorprone // to avoid using perfmark's dependency
+    compile (libraries.perfmark) {
+        exclude group: 'com.google.errorprone', module: 'error_prone_annotations'
+    }


Updated comment as "prefer our version to perfmark's 2.3.3".

suztomo · 2020-01-02T18:31:49Z

I see Android and Bazel build failed. Will look into these.

suztomo · 2020-01-02T19:02:47Z

@dapengzhang0 Would you run presubmit Kokoro check for the latest commit (ef42e8b)?

749b67f passed Bazel test.

suztomo · 2020-01-02T20:56:45Z

dapengzhang0 Thanks!

Now all checks have passed (13 successful checks). Would you merge this?

ejona86

Wow, this has been the most painful upgrade of Error Prone in a while. Thank you for the work. I think we can reduce the diff size considerably though with two small renames.

ejona86 · 2020-01-02T22:23:59Z

core/src/test/java/io/grpc/internal/ManagedChannelImplTest2.java

@@ -606,7 +606,7 @@ public void callsAndShutdownAndShutdownNow() {
  }

  private void subtestCallsAndShutdown(boolean shutdownNow, boolean shutdownNowAfterShutdown) {
-    FakeNameResolverFactory nameResolverFactory =
+    ManagedChannelImplTest2.FakeNameResolverFactory nameResolverFactory =


Instead of changing all of these names, could we instead just change the one other name. It looks like duplicate name is because of the class FakeNameResolverFactory on line 3743. Let's just change that one class's name. Worst-case, name it FakeNameResolverFactory2.

Ditto on line 3452 in ManagedChannelImplTest.java

…ions

suztomo

@ejona86 PTAL.

suztomo · 2020-01-03T15:52:24Z

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -3417,7 +3417,7 @@ public void start(Listener2 listener) {
      public void shutdown() {}
    }

-    final class FakeNameResolverFactory extends NameResolver.Factory {
+    final class FakeNameResolverFactory2 extends NameResolver.Factory {


Renamed to FakeNameResolverFactory2 to avoid SameNameButDifferent.

suztomo · 2020-01-03T15:53:02Z

core/src/test/java/io/grpc/internal/ManagedChannelImplTest2.java

@@ -606,7 +606,7 @@ public void callsAndShutdownAndShutdownNow() {
  }

  private void subtestCallsAndShutdown(boolean shutdownNow, boolean shutdownNowAfterShutdown) {
-    FakeNameResolverFactory nameResolverFactory =
+    ManagedChannelImplTest2.FakeNameResolverFactory nameResolverFactory =


That's a nice idea. Updated.

ejona86

Much smaller diff size!

ejona86 · 2020-01-03T22:50:44Z

@suztomo, thank you!

suztomo · 2020-01-03T22:57:37Z

Thank you for the review.

error_prone_annotation 2.3.4

0417f50

suztomo commented Dec 27, 2019

View reviewed changes

suztomo added 3 commits December 27, 2019 14:15

error_prone_annotations not from perfmark

c534c13

error prone core 2.3.3

f4e6c09

errorprone core 2.3.4

484531b

Declaring errorprone to replace perfmark's dependency

e7f9c6a

suztomo commented Dec 27, 2019

View reviewed changes

suztomo changed the title ~~Upgrade error_prone_annotations to 2.3.4~~ [WIP] Upgrade error_prone_annotations to 2.3.4 Dec 27, 2019

codyoss mentioned this pull request Dec 27, 2019

deps: update dependency com.google.guava:guava-bom to v28.2-android googleapis/java-firestore#58

Closed

1 task

elharo reviewed Dec 30, 2019

View reviewed changes

suztomo mentioned this pull request Dec 30, 2019

Add suppressions for GuardedBy violations #6566

Merged

ejona86 mentioned this pull request Dec 30, 2019

bazel: Support maven_install #6553

Merged

Merge remote-tracking branch 'origin/master' into error_prone_annotat…

d6b4454

…ions

suztomo added 2 commits December 30, 2019 15:18

Merge remote-tracking branch 'origin/master' into error_prone_annotat…

f7fa412

…ions

IO_GRPC_GRPC_JAVA_ARTIFACTS

ef4264e

SuppressWarnings("UnnecessaryAnonymousClass")

9e1c3a5

suztomo commented Dec 30, 2019

View reviewed changes

suztomo added 3 commits December 30, 2019 15:45

FormatMethod

51175e4

grpc-netty: @SuppressWarnings("UnnecessaryAnonymousClass")

f26fe6d

Removed unused argument

61fb9f2

suztomo added 5 commits December 31, 2019 00:46

SuppressWarning(PreferJavaTimeOverload)

6fb9bac

SuppressWarnings("PreferJavaTimeOverload") at build.gradle

fb4d7a6

SuppressWarnings("GuardedBy")

028bb6e

BinaryLogProviderTest for SameNameButDifferent

a2f4a83

prefer our version to perfmark's 2.3.3

0a2a76b

suztomo commented Dec 31, 2019

View reviewed changes

suztomo changed the title ~~[WIP] Upgrade error_prone_annotations to 2.3.4~~ Upgrade error_prone_annotations to 2.3.4 Dec 31, 2019

creamsoup added the kokoro:force-run Add this label to a PR to tell Kokoro to re-run all tests. Not generally necessary label Jan 2, 2020

grpc-kokoro removed the kokoro:force-run Add this label to a PR to tell Kokoro to re-run all tests. Not generally necessary label Jan 2, 2020

Fixed 'FormatMethod from an indirect dependency'

749b67f

dapengzhang0 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 2, 2020

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 2, 2020

SuppressWarnings("GuardedBy")

ef42e8b

dapengzhang0 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 2, 2020

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 2, 2020

ejona86 reviewed Jan 2, 2020

View reviewed changes

athakor mentioned this pull request Jan 3, 2020

feat: add enforce linkage checker googleapis/java-asset#51

Closed

suztomo added 2 commits January 3, 2020 09:53

Merge remote-tracking branch 'origin/master' into error_prone_annotat…

dbcdc4c

…ions

FakeNameResolverFactory2

10ec0cf

suztomo commented Jan 3, 2020

View reviewed changes

ejona86 approved these changes Jan 3, 2020

View reviewed changes

ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 3, 2020

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Jan 3, 2020

ejona86 merged commit 75f6fd8 into grpc:master Jan 3, 2020

lock bot locked as resolved and limited conversation to collaborators Apr 4, 2020

Upgrade error_prone_annotations to 2.3.4 #6574

Upgrade error_prone_annotations to 2.3.4 #6574

Conversation

suztomo commented Dec 27, 2019

Motivation

Choose a reason for hiding this comment

suztomo commented Dec 27, 2019

creamsoup commented Dec 27, 2019

suztomo commented Dec 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elharo left a comment

Choose a reason for hiding this comment

suztomo commented Dec 30, 2019

ejona86 commented Dec 30, 2019

suztomo commented Dec 30, 2019

ejona86 commented Dec 30, 2019

elharo commented Dec 30, 2019

ejona86 commented Dec 30, 2019

suztomo commented Dec 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo commented Jan 2, 2020

suztomo commented Jan 2, 2020 • edited

suztomo commented Jan 2, 2020 • edited

ejona86 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejona86 left a comment

Choose a reason for hiding this comment

ejona86 commented Jan 3, 2020

suztomo commented Jan 3, 2020

suztomo commented Jan 2, 2020 •

edited

suztomo commented Jan 2, 2020 •

edited