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

deps: update to Guava 29.0-jre #837

Merged
merged 5 commits into from
Apr 16, 2020
Merged

deps: update to Guava 29.0-jre #837

merged 5 commits into from
Apr 16, 2020

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Mar 23, 2020

@loosebazooka 28.2 marked several classes "Do Not Mock" which necessitated a surprisingly large number of changes.

@codecov
Copy link

codecov bot commented Mar 23, 2020

Codecov Report

Merging #837 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #837   +/-   ##
=========================================
  Coverage     78.45%   78.46%           
  Complexity      610      610           
=========================================
  Files            96       96           
  Lines          2409     2410    +1     
  Branches        282      282           
=========================================
+ Hits           1890     1891    +1     
  Misses          411      411           
  Partials        108      108           
Impacted Files Coverage Δ Complexity Δ
...ols/managedcloudsdk/command/AsyncByteConsumer.java 100.00% <100.00%> (ø) 8.00 <1.00> (ø)

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 03a4882...a2788d4. Read the comment docs.

Copy link
Contributor

@loosebazooka loosebazooka left a comment

Choose a reason for hiding this comment

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

Just a minor question.

}
result.setFuture(executorService.submit(() -> consumeBytes(inputStream)));
ListenableFuture<String> submit = executorService.submit(() -> consumeBytes(inputStream));
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to expand this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No semantic difference, but it makes refactoring, debugging, and updating easier. The more operations get crammed into a single statement the harder it becomes to read. In this case, it was difficult to demock without introducing local variables.

@loosebazooka
Copy link
Contributor

I think you need to rebase to pull in #838 to fix the cloud sdk on windows downgrade issue

@elharo elharo changed the title update to Guava 28.2-jre eps: update to Guava 29.0-jre Apr 16, 2020
@elharo elharo changed the title eps: update to Guava 29.0-jre deps: update to Guava 29.0-jre Apr 16, 2020
@elharo elharo merged commit cd77618 into master Apr 16, 2020
@elharo elharo deleted the guava branch April 16, 2020 13:56
JoeWang1127 pushed a commit that referenced this pull request Nov 29, 2023
* update to Guava 28.2-jre

* trivial change--http-->https-->to force CI to run again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants