Conversation
Change-Id: I651119d3ff975115c48dcc3dd8149ed070ad6f37
dorasun
requested review from
BenRKarl,
sarahcaseybot and
sundquist
and removed request for
sarahcaseybot
| # | ||
| # Returns: | ||
| # Ad group resource name. | ||
| def create_ad_group(client, customer_id, campaign_resource_name, customizer_attribute_resource_name) |
There was a problem hiding this comment.
The function was defined to accept four arguments, including one called customizer_attribute_resource_name. However, this parameter is not actually needed or used for creating an ad group is it?
There was a problem hiding this comment.
Ah oops, thanks for catching that! Updated
| # | ||
| # Returns: | ||
| # Ad group ad resource name. | ||
| def create_ad_group_ad(client, customer_id, ad_group_resource_name) |
There was a problem hiding this comment.
Doesn't this function does need the customizer_attribute_resource_name to customize the ad text. The function was defined to accept only three arguments.
Check me on this since I'm working in Python in my mind.
There was a problem hiding this comment.
I think you're right, the customizer_attribute_resource_name should be passed to this function, not to create_ad_group.
There was a problem hiding this comment.
You're both correct, sorry I missed that! Updated
| # | ||
| # Returns: | ||
| # Ad group ad resource name. | ||
| def create_ad_group_ad(client, customer_id, ad_group_resource_name) |
There was a problem hiding this comment.
I think you're right, the customizer_attribute_resource_name should be passed to this function, not to create_ad_group.
Change-Id: If70201bd75e6d53d3102e8c00bbd4070daaca8b4
4 participants
Change-Id: I651119d3ff975115c48dcc3dd8149ed070ad6f37