AdSense Management API v1.1 code samples

[sgomes, 2012-01-09 13:30:43]

Hey Joe,

Not sure if there's some mailing list I should be sending this to, instead of
sending it to you directly. Please let me know if you'd like me to assign
someone else to the review.

Thanks!

Cheers,
Sérgio

[jcgregorio_google, 2012-01-09 14:30:55]

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_account_t...
File samples/adsense/get_account_tree.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:36: account_id = 'INSERT_ACCOUNT_ID_HERE'
use gflags and make this a command-line option.

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_cl...
File samples/adsense/get_all_ad_clients_for_account.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_cl...
samples/adsense/get_all_ad_clients_for_account.py:37: account_id =
'INSERT_ACCOUNT_ID_HERE'
Make a gflag.

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_un...
File samples/adsense/get_all_ad_units_for_custom_channel.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_un...
samples/adsense/get_all_ad_units_for_custom_channel.py:38: account_id =
'INSERT_ACCOUNT_ID_HERE'
Make all gflags.

[sgomes, 2012-01-11 17:55:24]

Thanks! All comments addressed.

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_account_t...
File samples/adsense/get_account_tree.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:36: account_id = 'INSERT_ACCOUNT_ID_HERE'
On 2012/01/09 14:30:55, jcgregorio_google wrote:
> use gflags and make this a command-line option.

Done.

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_cl...
File samples/adsense/get_all_ad_clients_for_account.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_cl...
samples/adsense/get_all_ad_clients_for_account.py:37: account_id =
'INSERT_ACCOUNT_ID_HERE'
On 2012/01/09 14:30:55, jcgregorio_google wrote:
> Make a gflag.

Done.

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_un...
File samples/adsense/get_all_ad_units_for_custom_channel.py (right):

http://codereview.appspot.com/5488082/diff/2001/samples/adsense/get_all_ad_un...
samples/adsense/get_all_ad_units_for_custom_channel.py:38: account_id =
'INSERT_ACCOUNT_ID_HERE'
On 2012/01/09 14:30:55, jcgregorio_google wrote:
> Make all gflags.

Done.

[jcgregorio_google, 2012-01-12 14:08:03]

http://codereview.appspot.com/5488082/diff/4003/samples/adsense/generate_repo...
File samples/adsense/generate_report.py (right):

http://codereview.appspot.com/5488082/diff/4003/samples/adsense/generate_repo...
samples/adsense/generate_report.py:33: gflags.DEFINE_string('ad_client_id',
None,
Flag definitions go at the global scope.

[sgomes, 2012-01-12 15:53:07]

Thanks, suggestion applied across all files with the issue.

http://codereview.appspot.com/5488082/diff/4003/samples/adsense/generate_repo...
File samples/adsense/generate_report.py (right):

http://codereview.appspot.com/5488082/diff/4003/samples/adsense/generate_repo...
samples/adsense/generate_report.py:33: gflags.DEFINE_string('ad_client_id',
None,
On 2012/01/12 14:08:03, jcgregorio_google wrote:
> Flag definitions go at the global scope.

Fixed across all files.

[jcgregorio_google, 2012-01-18 13:43:44]

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
File samples/adsense/get_account_tree.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:18: This includes the full tree of
sub-accounts.
Blank line between the single line description and the rest of the description.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:20: Tags: accounts.get
Sometimes Tags: has a blank line before it, in other files it doesn't. Make
consistent across all files, or remove as I'm not sure what Tags: is supposed to
do.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:28: import sample_utils
fix order of imports

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_accou...
File samples/adsense/get_all_accounts.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_accou...
samples/adsense/get_all_accounts.py:24: import sys
order of imports

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_ad_un...
File samples/adsense/get_all_ad_units_for_custom_channel.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_ad_un...
samples/adsense/get_all_ad_units_for_custom_channel.py:25: import sys
order of imports

[sgomes, 2012-01-20 13:57:18]

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
File samples/adsense/get_account_tree.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:18: This includes the full tree of
sub-accounts.
On 2012/01/18 13:43:49, jcgregorio_google wrote:
> Blank line between the single line description and the rest of the
description.

Done.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:20: Tags: accounts.get
On 2012/01/18 13:43:49, jcgregorio_google wrote:
> Sometimes Tags: has a blank line before it, in other files it doesn't. Make
> consistent across all files, or remove as I'm not sure what Tags: is supposed
to
> do.

This is for future usage, for pulling all the samples for a specific method in
documentation. Adding one blank line behind it in all files.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:28: import sample_utils
On 2012/01/18 13:43:49, jcgregorio_google wrote:
> fix order of imports

Done.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_accou...
File samples/adsense/get_all_accounts.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_accou...
samples/adsense/get_all_accounts.py:24: import sys
On 2012/01/18 13:43:49, jcgregorio_google wrote:
> order of imports

Done.

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_ad_un...
File samples/adsense/get_all_ad_units_for_custom_channel.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_all_ad_un...
samples/adsense/get_all_ad_units_for_custom_channel.py:25: import sys
On 2012/01/18 13:43:49, jcgregorio_google wrote:
> order of imports

Done.

[jcgregorio_google, 2012-01-31 17:21:23]

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
File samples/adsense/get_account_tree.py (right):

http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
samples/adsense/get_account_tree.py:18: This includes the full tree of
sub-accounts.
Does this patch need to be refreshed? I don't see a blank line between lines 17
and 18.

On 2012/01/20 13:57:18, sgomes wrote:
> On 2012/01/18 13:43:49, jcgregorio_google wrote:
> > Blank line between the single line description and the rest of the
> description.
> 
> Done.

[sgomes, 2012-02-01 11:38:06]

Sorry, I thought you meant remove, rather than add. Done for all files.

---
On 2012/01/31 17:21:23, jcgregorio_google wrote:
>
http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
> File samples/adsense/get_account_tree.py (right):
> 
>
http://codereview.appspot.com/5488082/diff/5006/samples/adsense/get_account_t...
> samples/adsense/get_account_tree.py:18: This includes the full tree of
> sub-accounts.
> Does this patch need to be refreshed? I don't see a blank line between lines
17
> and 18.
> 
> On 2012/01/20 13:57:18, sgomes wrote:
> > On 2012/01/18 13:43:49, jcgregorio_google wrote:
> > > Blank line between the single line description and the rest of the
> > description.
> > 
> > Done.

[jcgregorio_google, 2012-02-07 18:58:50]

LGTM

[jcgregorio_google, 2012-02-07 19:32:18]

On 2012/02/07 18:58:50, jcgregorio_google wrote:
> LGTM

Committed in
http://code.google.com/p/google-api-python-client/source/detail?r=75895416612...