build: use errorprone's GuardedBy rather than javax's #2061
Conversation
|
I guess this is one of the effort to replace all Javax annotation usages? |
|
@blakeli0 Yes, this change is towards the goal. However, we use tools that still use the javax annotations (#1938 (comment)) and, fortunately, there's no known problems in using javax annotations so far. I don't think we will replace the javax annotations completely anytime soon. |
|
[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!
|
|
[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!
|
If this is the case, do we want to pursue more replacements of javax annotations at this moment? |
| @@ -35,6 +35,7 @@ | |||
| import com.google.api.gax.httpjson.HttpRequestRunnable.RunnableResult; | |||
| import com.google.api.gax.rpc.StatusCode; | |||
| import com.google.common.base.Preconditions; | |||
| import com.google.errorprone.annotations.concurrent.GuardedBy; | |||
There was a problem hiding this comment.
Is there a documentation that GuardedBy from errorprone and javax have the exact same bahavior?
There was a problem hiding this comment.
https://errorprone.info/bugpattern/GuardedBy
Note: there are a couple more annotations called @GuardedBy, including javax.annotation.concurrent.GuardedBy and org.checkerframework.checker.lock.qual.GuardedBy. The check recognizes those versions of the annotation, but we recommend using com.google.errorprone.annotations.concurrent.GuardedBy.
So errorprone treats the GuardedBy annotations from errorprone and javax in the same manner.
Thanks, this PR LGTM. What I'm concerning is that should we keep the issue #1938 in our ready-to-fix backlog. We should move it out if we think it will not be fixed anytime soon. |
|
Let's remove #1938 from the ready-to-fix backlog. |
|
I'll create child issues for each annotations. |
Towards #1938 . This pull request replaces
javax.annotation.concurrent.GuardedBywith errorprone's annotation. The official documentation https://github.com/google/error-prone/blob/master/docs/bugpattern/GuardedBy.md usescom.google.errorprone.annotations.concurrent.GuardedBy.To reviewers, if you wonder whether the GuardedBy annotation has any effect, see the Bazel build caught a bug I intentionally introduced in #2060.