landrito commented 6 years ago

Should this PR also remove the Google::Gax::Credentials class as well?

geigerj commented 6 years ago

@landrito Indeed, thanks for the catch! That actually means we need to regenerate the LRO client, as well. I'll update when this is ready for review.

geigerj commented 6 years ago

Credentials removed.

geigerj commented 6 years ago

Hmm... although the new Operations client doesn't seem to require googleauth, which seems wrong. (It also doesn't currently have test coverage, although I'd rather address that on follow-up than on this PR.) Taking a look.

geigerj commented 6 years ago

It looks like this is OK. (Requiring google/gax pulls in googleauth transitively through the grpc module.) CI failure seems to be due to gRPC 1.7.1 not supporting Ruby 2.0.0. @landrito PTAL?

landrito commented 6 years ago

I think it's better to not have a transitive dependency to googleauth: https://github.com/googleapis/toolkit/pull/1665

codecov-io commented 6 years ago

Codecov Report

Merging #97 into master will increase coverage by 0.78%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   98.57%   99.35%   +0.78%     
==========================================
  Files          20       18       -2     
  Lines        1819     1708     -111     
==========================================
- Hits         1793     1697      -96     
+ Misses         26       11      -15

Impacted Files	Coverage Δ
lib/google/gax.rb	`100% <ø> (ø)`	:arrow_up:

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 af04f40...24708a0. Read the comment docs.

googleapis / gax-ruby

Support googleauth 0.6.1 #97

Codecov Report