Support enumerations when generating types

[ianks]

I am currently working on a library which uses this gem extensively, and have ran across the need to have access to the possible values of an ENUMERABLE type. Before this patch, code-gen would generate a rather unhelpful type for enums. i.e.

registry.get_type_signature('CampaignStatus') #=> { :fields=>[] }

After this PR it will look like this:

registry.get_type_signature('CampaignStatus') #=> { :type => 'string', :enumerations => ['UNKNOWN' , 'ENABLED', 'PAUSED', 'REMOVED', :min_occurs => 0, :max_occurs => 1 }

I'm not entirely sure what the best way to expose this is, so this is my first attempt. I would love some feedback to make this better so we can get it merged!

Cheers, Ian

[ianks]

Also, I have a branch with the generated types available so you can look at them: https://github.com/googleads/google-api-ads-ruby/compare/master...ianks:enums-generated

[mcloonan]

This looks pretty useful; how extensively have you tested this? Are you still able to make successful API calls using that field? I'll see if we can run some tests using this code on our side to make sure it's safe before merging as well.

I'm curious what your use case is that you need to parse the registry yourself at runtime, as well.

[ianks]

This looks pretty useful; how extensively have you tested this?

I have tested it in a gem I have been working on, but nothing in a production env. It is currently making API calls successfully.

I'm curious what your use case is that you need to parse the registry yourself at runtime, as well.

The gem I am working on is making a rom-rb adapter for adwords, which will give a very ruby friendly interface over adwords. We are planning to open source it soon and I will ping you when that happens! :smile:

[donovanfm]

This looks like a really nice change. I tested this change for DFP and all of my API requests are working with the new registry files. I have a few minor comments:

1. Please document the new try_extract_enumeration function.
2. The use of seq_nodes seems to be a carry over from the other blocks in extract_type. I would prefer better variable names. Maybe restriction_node and enumeration_node?
3. Conventionally, we use explicit return statements. This applies in both extract_type and try_extract_enumeration.
4. The name try_extract_enumeration implies error handling to me. Would you be opposed to changing it to extract_enumerations for consistency with other methods in this class?

[ianks]

@donovanfm Thanks for your comments! They have been addressed. Let me know if you need anything else to merge this!

[donovanfm]

Thanks for the quick action! A few more comments:

1. Nitpick: can you change the documentation to read "an `enumerations` key".
2. Fix return typetype on line 169.
3. Break lines 161 and 165 after (type_element, to keep those line lengths under 80 characters. On the new line, indent 4 spaces from the previous line. See how it's done on line 144. (Sorry for not catching this one last time.)

[ianks]

@donovanfm I addessed your comments, can you take a last look?