Skip to content
This repository was archived by the owner on Jun 28, 2022. It is now read-only.

ResourceNameOneofConfig fixes #2704

Merged

Conversation

andreamlin
Copy link
Contributor

@andreamlin andreamlin commented Apr 8, 2019

When creating a ResourceNameOneofConfig from protofile annotations, use the already-created collections of SingleResourceNameConfigs and FixedResourceNameConfigs derived from the protofile annotations.

This will come in handy when we merge in GAPIC config overrides for resource names, because we should create the OneofConfigs from the overriden resource name configs, instead of making the child resource name configs from scratch from the protofile.

I know that there will be big changes to the ResourceSet design, but these bug fixes should still be relevant after the redesign too.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 8, 2019
@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #2704 into gapic_config_v2 will increase coverage by <.01%.
The diff coverage is 53.84%.

Impacted file tree graph

@@                  Coverage Diff                  @@
##             gapic_config_v2    #2704      +/-   ##
=====================================================
+ Coverage              86.82%   86.82%   +<.01%     
  Complexity              5602     5602              
=====================================================
  Files                    467      467              
  Lines                  22421    22420       -1     
  Branches                2433     2431       -2     
=====================================================
+ Hits                   19466    19467       +1     
+ Misses                  2094     2092       -2     
  Partials                 861      861
Impacted Files Coverage Δ Complexity Δ
.../google/api/codegen/config/GapicProductConfig.java 79.11% <100%> (ø) 92 <0> (ø) ⬇️
...le/api/codegen/config/ResourceNameOneofConfig.java 53.12% <42.85%> (+2.35%) 10 <3> (ø) ⬇️

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 5be1b6d...00f9ab0. Read the comment docs.

@andreamlin andreamlin changed the title diff ResourceSet parsing Apr 9, 2019
@andreamlin andreamlin changed the title ResourceSet parsing ResourceNameOneofConfig fixes Apr 9, 2019
@andreamlin andreamlin requested a review from michaelbausor April 9, 2019 00:11
@andreamlin
Copy link
Contributor Author

PTAL

@andreamlin andreamlin merged commit 28e8372 into googleapis:gapic_config_v2 Apr 9, 2019
@andreamlin andreamlin deleted the order_resource_names branch April 9, 2019 16:11
andreamlin added a commit that referenced this pull request Apr 22, 2019
* Add Gapic config v2 (#2665)
* Whittling down config_v2 (#2666)
* Add ConfigV2 Validator (#2672)
* AutoValue LongRunningConfig; always use gapic config's polling settings (#2698)
* ResourceNameOneofConfig fixes (#2704)
* Start parsing GAPIC config v2 (#2703)
* Bring back timeout millis in GAPIC config v2 (#2708)
* Resource names across different protofiles (#2711)
* Fix missing default retries (#2718)
* Bug fixes for gapic config v2 parsing (#2717)
busunkim96 pushed a commit to busunkim96/gapic-generator that referenced this pull request Nov 7, 2019
* Add Gapic config v2 (googleapis#2665)
* Whittling down config_v2 (googleapis#2666)
* Add ConfigV2 Validator (googleapis#2672)
* AutoValue LongRunningConfig; always use gapic config's polling settings (googleapis#2698)
* ResourceNameOneofConfig fixes (googleapis#2704)
* Start parsing GAPIC config v2 (googleapis#2703)
* Bring back timeout millis in GAPIC config v2 (googleapis#2708)
* Resource names across different protofiles (googleapis#2711)
* Fix missing default retries (googleapis#2718)
* Bug fixes for gapic config v2 parsing (googleapis#2717)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants