googleapis / gax-ruby

Google API Extensions for Ruby
https://rubygems.org/gems/google-gax
BSD 3-Clause "New" or "Revised" License
20 stars 22 forks source link

Remove params_extractor #175

Closed blowmage closed 5 years ago

blowmage commented 5 years ago

[closes #173]

codecov[bot] commented 5 years ago

Codecov Report

Merging #175 into master will decrease coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #175      +/-   ##
=========================================
- Coverage   98.61%   98.6%   -0.01%     
=========================================
  Files          25      25              
  Lines        1441    1432       -9     
=========================================
- Hits         1421    1412       -9     
  Misses         20      20
Impacted Files Coverage Δ
lib/google/gax/api_call.rb 97.14% <100%> (-0.59%) :arrow_down:

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 8436f0a...f502c01. Read the comment docs.

jbolinger commented 5 years ago

Let's update https://github.com/googleapis/gapic-generator-ruby/issues/104 and/or open a PR in the generator to accompany this one so we can see how the usage of this will look from the client side.

blowmage commented 5 years ago

I have updated googleapis/gapic-generator-ruby#94 to be more up to date with the changes made in the GAPIC generator and in Gax. It removes params_extractor from Gax entirely, and demonstrates how the GAPIC generator client can provide that type of functionality.

jbolinger commented 5 years ago

@blowmage I did a quick pass on https://github.com/googleapis/gapic-generator-ruby/pull/94, but it looks like this functionality is still a WIP?

blowmage commented 5 years ago

googleapis/gapic-generator-ruby#94 is a working branch that demonstrates that features can be moved from Gax to the generator. Much of what that branch is using is available to review in #178. Not all of it, but most of it.

blowmage commented 5 years ago

This is satisfied by #178.