Skip to content

Conversation

@justinuang
Copy link

@justinuang justinuang commented Oct 10, 2019

Some notes:

  • auto-value-annotations should be a compile time dependency: https://github.com/google/auto/blob/master/value/userguide/index.md#in-pomxml

  • BaseBigtableInstanceAdminSettings uses iam protos

  • Remove dependency on com.google.http-client:google-http-client because of a wrong import of Lists

  • We have to specify the version of threetenbp = 1.3.3 to match gax, as enforced by the dependency convergence rule.

  • jsr305 is used to provide javax.annotations.Nonnull, etc. These are added to java in java 8, but since we need to be compatible with java 7, we have an explicit import.

@igorbernstein2

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 10, 2019
Copy link

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

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

Thanks for digging into this! This looks great, but I think can be a improved a bit

@codecov
Copy link

codecov bot commented Oct 10, 2019

Codecov Report

Merging #6484 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6484      +/-   ##
============================================
+ Coverage     46.34%   46.34%   +<.01%     
- Complexity    27974    28009      +35     
============================================
  Files          2613     2613              
  Lines        287955   288056     +101     
  Branches      33756    33769      +13     
============================================
+ Hits         133456   133512      +56     
- Misses       144279   144306      +27     
- Partials      10220    10238      +18
Impacted Files Coverage Δ Complexity Δ
...curityscanner/v1beta/WebSecurityScannerClient.java 66.5% <0%> (-16.04%) 55% <0%> (+7%)
...src/main/java/com/google/cloud/storage/Bucket.java 81.22% <0%> (-0.6%) 34% <0%> (ø)
...main/java/com/google/cloud/storage/BucketInfo.java 84.36% <0%> (-0.5%) 84% <0%> (+3%)
...om/google/cloud/bigquery/CopyJobConfiguration.java 97.32% <0%> (+0.26%) 27% <0%> (+2%) ⬆️
...om/google/cloud/bigquery/LoadJobConfiguration.java 89.71% <0%> (+0.44%) 50% <0%> (+2%) ⬆️
...google/cloud/bigquery/ExtractJobConfiguration.java 94.49% <0%> (+0.55%) 27% <0%> (+2%) ⬆️
...m/google/cloud/bigquery/QueryJobConfiguration.java 75.68% <0%> (+0.84%) 52% <0%> (+2%) ⬆️
...oogle/cloud/spanner/jdbc/SingleUseTransaction.java 87.5% <0%> (+1%) 37% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b8acda...64032b9. Read the comment docs.

Copy link

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

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

This looks great, except for a few nits. Other then couple of inline comments, can you also run a diff of mvn dependency:list of the old Pom and the new? And post it in the description of the pr for future reference?

Copy link
Author

@justinuang justinuang left a comment

Choose a reason for hiding this comment

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

justinuang@justinuang1:~/code/google-cloud-java$ diff -U -1 {old,new}_dependencies.txt
--- old_dependencies.txt	2019-10-14 17:21:25.522738087 -0400
+++ new_dependencies.txt	2019-10-14 17:21:13.846827193 -0400
@@ -1,66 +1,66 @@

 The following files have been resolved:
    com.fasterxml.jackson.core:jackson-core:jar:2.9.9:compile
    com.google.android:annotations:jar:4.1.1.4:compile
    com.google.api.grpc:grpc-google-cloud-bigtable-admin-v2:jar:0.79.0:test
    com.google.api.grpc:grpc-google-cloud-bigtable-v2:jar:0.79.0:test
    com.google.api.grpc:proto-google-cloud-bigtable-admin-v2:jar:0.79.0:compile
    com.google.api.grpc:proto-google-cloud-bigtable-v2:jar:0.79.0:compile
    com.google.api.grpc:proto-google-cloud-firestore-v1:jar:1.26.0:test
    com.google.api.grpc:proto-google-common-protos:jar:1.16.0:compile
    com.google.api.grpc:proto-google-iam-v1:jar:0.13.0:compile
    com.google.api:api-common:jar:1.8.1:compile
    com.google.api:gax-grpc:jar:1.49.0:compile
    com.google.api:gax-grpc:jar:testlib:1.49.0:test
    com.google.api:gax:jar:1.49.0:compile
    com.google.api:gax:jar:testlib:1.49.0:test
    com.google.auth:google-auth-library-credentials:jar:0.17.2:compile
    com.google.auth:google-auth-library-oauth2-http:jar:0.17.2:compile
    com.google.auto.value:auto-value-annotations:jar:1.6.6:compile
    com.google.auto.value:auto-value:jar:1.6.6:provided
    com.google.cloud:google-cloud-bigtable-emulator:jar:0.114.0-alpha:test
    com.google.cloud:google-cloud-conformance-tests:jar:0.1.0-SNAPSHOT:test
    com.google.cloud:google-cloud-core-grpc:jar:1.90.0:compile
    com.google.cloud:google-cloud-core:jar:1.90.0:compile
    com.google.code.findbugs:jsr305:jar:3.0.2:compile
    com.google.code.gson:gson:jar:2.8.5:compile
    com.google.errorprone:error_prone_annotations:jar:2.3.3:compile
    com.google.guava:failureaccess:jar:1.0.1:compile
    com.google.guava:guava:jar:28.1-android:compile
    com.google.guava:listenablefuture:jar:9999.0-empty-to-avoid-conflict-with-guava:compile
    com.google.http-client:google-http-client-jackson2:jar:1.32.1:compile
    com.google.http-client:google-http-client:jar:1.32.1:compile
    com.google.j2objc:j2objc-annotations:jar:1.3:compile
    com.google.protobuf:protobuf-java-util:jar:3.10.0:compile
    com.google.protobuf:protobuf-java:jar:3.10.0:compile
    com.google.truth:truth:jar:1.0:test
    com.googlecode.java-diff-utils:diffutils:jar:1.3.0:test
    commons-codec:commons-codec:jar:1.11:compile
    commons-logging:commons-logging:jar:1.2:compile
-   io.grpc:grpc-alts:jar:1.24.0:compile
+   io.grpc:grpc-alts:jar:1.24.0:runtime
    io.grpc:grpc-api:jar:1.24.0:compile
    io.grpc:grpc-auth:jar:1.24.0:compile
    io.grpc:grpc-context:jar:1.24.0:compile
    io.grpc:grpc-core:jar:1.24.0:compile
-   io.grpc:grpc-grpclb:jar:1.24.0:compile
+   io.grpc:grpc-grpclb:jar:1.24.0:runtime
    io.grpc:grpc-netty-shaded:jar:1.24.0:compile
    io.grpc:grpc-protobuf-lite:jar:1.24.0:compile
    io.grpc:grpc-protobuf:jar:1.24.0:compile
-   io.grpc:grpc-stub:jar:1.24.0:compile
+   io.grpc:grpc-stub:jar:1.24.0:test
    io.grpc:grpc-testing:jar:1.24.0:test
    io.opencensus:opencensus-api:jar:0.24.0:compile
    io.opencensus:opencensus-contrib-grpc-metrics:jar:0.21.0:compile
    io.opencensus:opencensus-contrib-http-util:jar:0.24.0:compile
    io.perfmark:perfmark-api:jar:0.17.0:compile
    javax.annotation:javax.annotation-api:jar:1.3.2:compile
    junit:junit:jar:4.12:test
-   org.apache.commons:commons-lang3:jar:3.5:compile
+   org.apache.commons:commons-lang3:jar:3.5:runtime
    org.apache.httpcomponents:httpclient:jar:4.5.10:compile
    org.apache.httpcomponents:httpcore:jar:4.4.12:compile
    org.checkerframework:checker-compat-qual:jar:2.5.5:compile
    org.codehaus.mojo:animal-sniffer-annotations:jar:1.18:compile
    org.hamcrest:hamcrest-core:jar:2.1:test
    org.hamcrest:hamcrest:jar:2.1:test
    org.mockito:mockito-all:jar:1.10.19:test
    org.threeten:threetenbp:jar:1.3.3:compile

@kolea2
Copy link
Contributor

kolea2 commented Oct 15, 2019

Thanks for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants