Skip to content

Animal-sniffer-annotation 1.18#6488

Merged
creamsoup merged 4 commits intogrpc:masterfrom
suztomo:animal_sniffer
Dec 5, 2019
Merged

Animal-sniffer-annotation 1.18#6488
creamsoup merged 4 commits intogrpc:masterfrom
suztomo:animal_sniffer

Conversation

@suztomo
Copy link
Copy Markdown
Contributor

@suztomo suztomo commented Dec 4, 2019

Upgrading animal-sniffer-annotation to the latest 1.18.

There was a problem in upperBoundCheck in google-cloud-firestore: googleapis/java-firestore#34.

@creamsoup creamsoup added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 5, 2019
@grpc-kokoro 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 Dec 5, 2019
@creamsoup
Copy link
Copy Markdown
Contributor

creamsoup commented Dec 5, 2019

bazel dep needs to be updated, see repositories.bzl.

nit: please update comments mentioning the old version

api/build.gradle:        // prefer 1.17 from libraries instead of 1.14
protobuf-lite/build.gradle:        // prefer 1.17 from libraries instead of 1.14
protobuf/build.gradle:        // prefer 1.17 from libraries instead of 1.14

@creamsoup creamsoup self-requested a review December 5, 2019 01:46
suztomo
suztomo commented Dec 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@suztomo suztomo left a comment

Choose a reason for hiding this comment

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

@creamsoup Thank you for checking. Updated bzl file.

Comment thread api/build.gradle Outdated
// prefer 3.0.2 from libraries instead of 3.0.1
exclude group: 'com.google.code.findbugs', module: 'jsr305'
// prefer 1.17 from libraries instead of 1.14
// prefer our own version from libraries instead of Guava's dependency
Copy link
Copy Markdown
Contributor Author

@suztomo suztomo Dec 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Guava 28.1 has dependency animal-sniffer-annotation 1.18, not 1.14. The old comment was outdated; it must have been hard to maintain the comment precisely by reading the Guava's pom.xml.

@creamsoup I updated this comment in more generic way, and now I feel these comments "prefer XXX from libraries instead of YYY" are hard to maintain and also my "prefer our own version from libraries instead of Guava's dependency" looks unnecessary. Don't you think we should remove these comments?

Copy link
Copy Markdown
Contributor

@creamsoup creamsoup Dec 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

yeah it is hard to maintain, i like your new comment (or no comment is also good enough).
i am going to make the cleanup PR. thanks for the suggestion!

Comment thread repositories.bzl
artifact = "org.codehaus.mojo:animal-sniffer-annotations:1.18",
server_urls = ["http://central.maven.org/maven2"],
artifact_sha256 = "92654f493ecfec52082e76354f0ebf87648dc3d5cec2e3c3cdb947c016747a53",
artifact_sha256 = "47f05852b48ee9baefef80fa3d8cea60efa4753c0013121dd7fe5eef2e5c729d",
Copy link
Copy Markdown
Contributor Author

@suztomo suztomo Dec 5, 2019

Choose a reason for hiding this comment

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

suztomo-macbookpro44:grpc-java suztomo$ shasum -a 256 ~/Downloads/animal-sniffer-annotations-1.18.jar 
47f05852b48ee9baefef80fa3d8cea60efa4753c0013121dd7fe5eef2e5c729d  /Users/suztomo/Downloads/animal-sniffer-annotations-1.18.jar

creamsoup
creamsoup approved these changes Dec 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@creamsoup creamsoup left a comment

Choose a reason for hiding this comment

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

LGTM

@creamsoup creamsoup added the kokoro:force-run Add this label to a PR to tell Kokoro to re-run all tests. Not generally necessary label Dec 5, 2019
@grpc-kokoro 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 Dec 5, 2019
@suztomo
Copy link
Copy Markdown
Contributor Author

suztomo commented Dec 5, 2019

Updated the comments to one generic comment for the occurrences of animal-sniffer-annotations exclusions.

suztomo
suztomo commented Dec 5, 2019
View reviewed changes
Comment thread api/build.gradle
libraries.jsr305,
libraries.animalsniffer_annotations
compile (libraries.guava) {
// prefer 2.3.3 from libraries instead of 2.1.3
Copy link
Copy Markdown
Contributor Author

@suztomo suztomo Dec 5, 2019

Choose a reason for hiding this comment

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

This comment has been outdated; Guava 28.1 uses error_prone_annotations:2.3.2.

Comment thread api/build.gradle
// prefer 2.3.3 from libraries instead of 2.1.3
// prefer our own versions from libraries instead of Guava's dependency
exclude group: 'com.google.errorprone', module: 'error_prone_annotations'
// prefer 3.0.2 from libraries instead of 3.0.1
Copy link
Copy Markdown
Contributor Author

@suztomo suztomo Dec 5, 2019

Choose a reason for hiding this comment

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

This comment has been outdated; Guava 28.1 uses jsr305:3.0.2.

@suztomo suztomo mentioned this pull request Dec 5, 2019
Merged
@suztomo
Copy link
Copy Markdown
Contributor Author

suztomo commented Dec 5, 2019

Created another PR that for files that are irrelevant to animal-sniffer-annotations.
#6492

@ericgribkoff ericgribkoff added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 5, 2019
@grpc-kokoro 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 Dec 5, 2019
@creamsoup creamsoup merged commit 80699f8 into grpc:master Dec 5, 2019
@creamsoup
Copy link
Copy Markdown
Contributor

creamsoup commented Dec 5, 2019

merged, thanks @suztomo!

@suztomo
Copy link
Copy Markdown
Contributor Author

suztomo commented Dec 5, 2019

Thank you.

@suztomo suztomo deleted the animal_sniffer branch December 5, 2019 21:17
@lock lock Bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@creamsoup creamsoup creamsoup approved these changes

@ericgribkoff ericgribkoff ericgribkoff approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@suztomo @creamsoup @ericgribkoff @grpc-kokoro