Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ImmutableMap.Builder.build() with buildOrThrow() #10648

Conversation

findepi
Copy link
Member

@findepi findepi commented Jan 17, 2022

Starting with Guava 31, the build() method is discouraged and will be
deprecated in upcoming releases. See
google/guava@4bbe12c
for context.

@findepi findepi requested review from losipiuk and kokosing January 17, 2022 19:44
@cla-bot cla-bot bot added the cla-signed label Jan 17, 2022
@findepi findepi force-pushed the findepi/replace-immutablemap-builder-build-with-buildorthrow-f30a90 branch 3 times, most recently from a019768 to ab477ab Compare January 17, 2022 21:28
@findepi
Copy link
Member Author

findepi commented Jan 17, 2022

Interesting. For the first commit, Github reports +924 −935 for line changes, which is disturbing, as line count shouldn't change.
Locally, git show --shortstat reports 494 files changed, 921 insertions(+), 924 deletions(-), so 3 lines less.
And indeed, in 3 files a .build() was preceded with an unnecessary line break

$ git show --numstat | grep '^[0-9]' | grep -v '^\([0-9]\+\)\s\1'
36	37	plugin/trino-elasticsearch/src/test/java/io/trino/plugin/elasticsearch/BaseElasticsearchConnectorTest.java
2	3	testing/trino-benchmark/src/main/java/io/trino/benchmark/AbstractOperatorBenchmark.java
3	4	testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveCoercion.java

This explains the the # line change as reported by git. Where the GitHub UI number comes from, this I don't know.

@github-actions github-actions bot added jdbc Relates to Trino JDBC driver tests:hive labels Jan 17, 2022
@losipiuk
Copy link
Member

red

@@ -40,11 +40,11 @@

public BackupModule(Map<String, Module> providers)
{
this.providers = ImmutableMap.<String, Module>builder()
this.providers = ImmutableMap.<String, java.lang.Module>builder()
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bad change done by structural replace in the IDE.
The Module used here is com.google.inject.Module, not java.lang.Module.

Starting with Guava 31, the `build()` method is discouraged and will be
deprecated in upcoming releases. See
google/guava@4bbe12c
for context.

The change itself was done mechanically, with IntelliJ's structural
search, then fixed line breaks with regular search for
`^(\s+)(.put(All)?\(.*\))(\.buildOrThrow\(\))`, then occurrences of
`).buildOrThrow()` fixed manually, then end-of-line searched for in the
diff (as they got removed by the structural search/replace) and also
restored manually.
@findepi findepi force-pushed the findepi/replace-immutablemap-builder-build-with-buildorthrow-f30a90 branch from ab477ab to d8efa20 Compare January 18, 2022 10:32
@findepi findepi merged commit 3e06ac2 into trinodb:master Jan 18, 2022
@findepi findepi deleted the findepi/replace-immutablemap-builder-build-with-buildorthrow-f30a90 branch January 18, 2022 21:32
@github-actions github-actions bot added this to the 369 milestone Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed jdbc Relates to Trino JDBC driver
Development

Successfully merging this pull request may close these issues.

2 participants