Remove dependence on gflags

Remove dependence on gflags.

Fixes issue #246.

[jcgregorio_google, 2013-03-12 20:21:54]

Note that this CL doesn't update all the samples yet, it only updated plus.py to
show how all the samples will be changed.

[dhermes, 2013-03-12 21:12:33]

Is there anything I should do to:

1) Make sure tests run?

2) Make sure this is comprehensive? Could you provide an idea of what you
"grep"-ed for or other things you did.

3) Follow up with the samples other than plus?

https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py
File apiclient/model.py (right):

https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py#newcode34
apiclient/model.py:34: dump_request_response = False
What about using argparse here?

I see the change in tests/test_json_model.py to reflect how this has changed,
but am not sure this is "good enough" to not break users?

https://codereview.appspot.com/7628044/diff/6001/describe.py
File describe.py (right):

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode140
describe.py:140: 'this API will be documented.')
Can you put parens around this string and line the string up together on the two
lines?

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode144
describe.py:144: ' is supplied.')
Can you put parens around this string and line the string up together on the two
lines?

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode149
describe.py:149: FLAGS = parser.parse_args()
This should go in "if __name__ == '__main__'" and take sys.argv

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode227
describe.py:227: return string.Template(METHOD_TEMPLATE).substitute(name=name,
params=params, doc=doc)
Line length

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode295
describe.py:295:
html.append(string.Template(COLLECTION_LINK).substitute(href=href, name=name))
Line length

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py
File expand-symlinks.py (right):

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode1
expand-symlinks.py:1: #!/usr/bin/python2.4
This is a module which will never be importable. Is the filename with a '-'
character intentional?

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode42
expand-symlinks.py:42: FLAGS = parser.parse_args()
This should happen in main.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py
File oauth2client/tools.py (right):

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode50
oauth2client/tools.py:50: help='Host name to use when running a local web server
to '
Bad indent.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode52
oauth2client/tools.py:52: run_parser.add_argument('--noauth_local_webserver',
action='store_true', default=False,
Line length. Why the n in front of auth_local_webserver?

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode56
oauth2client/tools.py:56: nargs='*', help='Port to use when running a local web
server to '
Line length

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode59
oauth2client/tools.py:59: choices=['DEBUG', 'INFO', 'WARNING', 'ERROR',
'CRITICAL'],
Bad indent

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
oauth2client/tools.py:104: def run(flow, storage, flags, http=None):
IMO it's safer to make flags a required keyword arg than it is to add a
positional arg. Up to you.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
oauth2client/tools.py:227: return """WARNING: Please configure OAuth 2.0
Make this template a top-level constant.

https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py
File samples/plus/plus.py (right):

https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py#newcode83
samples/plus/plus.py:83: while (request != None):
You can lose the parens all together. Also, style guide says use "is" or "is
not" for None comparisons, not "!=".

https://codereview.appspot.com/7628044/diff/6001/setup.py
File setup.py (left):

https://codereview.appspot.com/7628044/diff/6001/setup.py#oldcode64
setup.py:64: scripts=['bin/enable-app-engine-project'],
Did you mean to axe this?

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py
File tests/test_discovery.py (right):

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:77: positional_parameters_enforcement =
POSITIONAL_EXCEPTION
This won't do what you think it's doing. You need to change it on the module.

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:334: discovery = open(datafile('plus.json')).read()
Why this change?

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:505: ({'status': '200'},
b'echo_request_headers_as_json'),
Python3 compatibility stuff creeping into this change?

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:1035: self.assertEqual(expected,
simplejson.loads(e.content.decode('utf-8')),
Why this? I'm going to assume this is also part of the Python3 stuff. We should
not commingle the two sets of changes.

[jcgregorio_google, 2013-03-13 13:47:49]

https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py
File apiclient/model.py (right):

https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py#newcode34
apiclient/model.py:34: dump_request_response = False
The argparse module doesn't work like gflags in that way, i.e. you can't define
global flags, you have to have the instance of argparse.ArgumentParser() to add
the flags to. 

One possibility is to define an apiclient ArgumentParser instance that flags are
added to, then you can pass that as a parent ArgumentParser to the applications
ArgumentParser, like what's done in plus.py.

That seems like overkill for a single flag, but it would keep us more backward
compliant.

On 2013/03/12 21:12:33, dhermes wrote:
> What about using argparse here?
> 
> I see the change in tests/test_json_model.py to reflect how this has changed,
> but am not sure this is "good enough" to not break users?

[jcgregorio_google, 2013-03-13 15:22:49]

On 2013/03/12 21:12:33, dhermes wrote:
> Is there anything I should do to:
> 
> 1) Make sure tests run?

If you patch this in please make sure to run

$ tox -r

to force reinstallation of a virtualenv that doesn't contain gflags.

> 
> 2) Make sure this is comprehensive? Could you provide an idea of what you
> "grep"-ed for or other things you did.

grep -r gflags *

> 
> 3) Follow up with the samples other than plus?

Once this CL is good I will modify all the other samples.

> 
> https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py
> File apiclient/model.py (right):
> 
> https://codereview.appspot.com/7628044/diff/6001/apiclient/model.py#newcode34
> apiclient/model.py:34: dump_request_response = False
> What about using argparse here?
> 
> I see the change in tests/test_json_model.py to reflect how this has changed,
> but am not sure this is "good enough" to not break users?
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py
> File describe.py (right):
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode140
> describe.py:140: 'this API will be documented.')
> Can you put parens around this string and line the string up together on the
two
> lines?
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode144
> describe.py:144: ' is supplied.')
> Can you put parens around this string and line the string up together on the
two
> lines?
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode149
> describe.py:149: FLAGS = parser.parse_args()
> This should go in "if __name__ == '__main__'" and take sys.argv
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode227
> describe.py:227: return string.Template(METHOD_TEMPLATE).substitute(name=name,
> params=params, doc=doc)
> Line length
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode295
> describe.py:295:
> html.append(string.Template(COLLECTION_LINK).substitute(href=href, name=name))
> Line length
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py
> File expand-symlinks.py (right):
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode1
> expand-symlinks.py:1: #!/usr/bin/python2.4
> This is a module which will never be importable. Is the filename with a '-'
> character intentional?
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode42
> expand-symlinks.py:42: FLAGS = parser.parse_args()
> This should happen in main.
> 
> https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py
> File oauth2client/tools.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode50
> oauth2client/tools.py:50: help='Host name to use when running a local web
server
> to '
> Bad indent.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode52
> oauth2client/tools.py:52: run_parser.add_argument('--noauth_local_webserver',
> action='store_true', default=False,
> Line length. Why the n in front of auth_local_webserver?
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode56
> oauth2client/tools.py:56: nargs='*', help='Port to use when running a local
web
> server to '
> Line length
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode59
> oauth2client/tools.py:59: choices=['DEBUG', 'INFO', 'WARNING', 'ERROR',
> 'CRITICAL'],
> Bad indent
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
> oauth2client/tools.py:104: def run(flow, storage, flags, http=None):
> IMO it's safer to make flags a required keyword arg than it is to add a
> positional arg. Up to you.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
> oauth2client/tools.py:227: return """WARNING: Please configure OAuth 2.0
> Make this template a top-level constant.
> 
> https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py
> File samples/plus/plus.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py#newcode83
> samples/plus/plus.py:83: while (request != None):
> You can lose the parens all together. Also, style guide says use "is" or "is
> not" for None comparisons, not "!=".
> 
> https://codereview.appspot.com/7628044/diff/6001/setup.py
> File setup.py (left):
> 
> https://codereview.appspot.com/7628044/diff/6001/setup.py#oldcode64
> setup.py:64: scripts=['bin/enable-app-engine-project'],
> Did you mean to axe this?
> 
> https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py
> File tests/test_discovery.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:77: positional_parameters_enforcement =
> POSITIONAL_EXCEPTION
> This won't do what you think it's doing. You need to change it on the module.
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:334: discovery = open(datafile('plus.json')).read()
> Why this change?
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:505: ({'status': '200'},
> b'echo_request_headers_as_json'),
> Python3 compatibility stuff creeping into this change?
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:1035: self.assertEqual(expected,
> simplejson.loads(e.content.decode('utf-8')),
> Why this? I'm going to assume this is also part of the Python3 stuff. We
should
> not commingle the two sets of changes.

[jcgregorio_google, 2013-03-13 15:22:58]

https://codereview.appspot.com/7628044/diff/6001/describe.py
File describe.py (right):

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode140
describe.py:140: 'this API will be documented.')
On 2013/03/12 21:12:33, dhermes wrote:
> Can you put parens around this string and line the string up together on the
two
> lines?

Done.

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode144
describe.py:144: ' is supplied.')
On 2013/03/12 21:12:33, dhermes wrote:
> Can you put parens around this string and line the string up together on the
two
> lines?

Done.

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode149
describe.py:149: FLAGS = parser.parse_args()
On 2013/03/12 21:12:33, dhermes wrote:
> This should go in "if __name__ == '__main__'" and take sys.argv

Done.

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode227
describe.py:227: return string.Template(METHOD_TEMPLATE).substitute(name=name,
params=params, doc=doc)
On 2013/03/12 21:12:33, dhermes wrote:
> Line length

Done.

https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode295
describe.py:295:
html.append(string.Template(COLLECTION_LINK).substitute(href=href, name=name))
On 2013/03/12 21:12:33, dhermes wrote:
> Line length

Done.

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py
File expand-symlinks.py (right):

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode1
expand-symlinks.py:1: #!/usr/bin/python2.4
Renamed to expandsymlinks.py.

On 2013/03/12 21:12:33, dhermes wrote:
> This is a module which will never be importable. Is the filename with a '-'
> character intentional?

https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode42
expand-symlinks.py:42: FLAGS = parser.parse_args()
On 2013/03/12 21:12:33, dhermes wrote:
> This should happen in main.

Done.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py
File oauth2client/tools.py (right):

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode50
oauth2client/tools.py:50: help='Host name to use when running a local web server
to '
On 2013/03/12 21:12:33, dhermes wrote:
> Bad indent.

Done.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode52
oauth2client/tools.py:52: run_parser.add_argument('--noauth_local_webserver',
action='store_true', default=False,
On 2013/03/12 21:12:33, dhermes wrote:
> Line length. Why the n in front of auth_local_webserver?

To keep the flag name compatible with what came out of gflags, which was
--noauth_local_webserver to turn the server off.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode56
oauth2client/tools.py:56: nargs='*', help='Port to use when running a local web
server to '
On 2013/03/12 21:12:33, dhermes wrote:
> Line length

Done.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode59
oauth2client/tools.py:59: choices=['DEBUG', 'INFO', 'WARNING', 'ERROR',
'CRITICAL'],
On 2013/03/12 21:12:33, dhermes wrote:
> Bad indent

Done.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
oauth2client/tools.py:104: def run(flow, storage, flags, http=None):
Acknowledged.

On 2013/03/12 21:12:33, dhermes wrote:
> IMO it's safer to make flags a required keyword arg than it is to add a
> positional arg. Up to you.

https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
oauth2client/tools.py:227: return """WARNING: Please configure OAuth 2.0
On 2013/03/12 21:12:33, dhermes wrote:
> Make this template a top-level constant.

Done.

https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py
File samples/plus/plus.py (right):

https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py#newcode83
samples/plus/plus.py:83: while (request != None):
On 2013/03/12 21:12:33, dhermes wrote:
> You can lose the parens all together. Also, style guide says use "is" or "is
> not" for None comparisons, not "!=".

Done.

https://codereview.appspot.com/7628044/diff/6001/setup.py
File setup.py (left):

https://codereview.appspot.com/7628044/diff/6001/setup.py#oldcode64
setup.py:64: scripts=['bin/enable-app-engine-project'],
Yes, it's no longer documented as the way to install to App Engine:

  https://developers.google.com/api-client-library/python/start/installation

On 2013/03/12 21:12:33, dhermes wrote:
> Did you mean to axe this?

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py
File tests/test_discovery.py (right):

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:77: positional_parameters_enforcement =
POSITIONAL_EXCEPTION
On 2013/03/12 21:12:33, dhermes wrote:
> This won't do what you think it's doing. You need to change it on the module.

Done.

https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
tests/test_discovery.py:1035: self.assertEqual(expected,
simplejson.loads(e.content.decode('utf-8')),
On 2013/03/12 21:12:33, dhermes wrote:
> Why this? I'm going to assume this is also part of the Python3 stuff. We
should
> not commingle the two sets of changes.

Done.

[jcgregorio_google, 2013-03-18 17:04:16]

ping

On 2013/03/13 15:22:58, jcgregorio_google wrote:
> https://codereview.appspot.com/7628044/diff/6001/describe.py
> File describe.py (right):
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode140
> describe.py:140: 'this API will be documented.')
> On 2013/03/12 21:12:33, dhermes wrote:
> > Can you put parens around this string and line the string up together on the
> two
> > lines?
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode144
> describe.py:144: ' is supplied.')
> On 2013/03/12 21:12:33, dhermes wrote:
> > Can you put parens around this string and line the string up together on the
> two
> > lines?
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode149
> describe.py:149: FLAGS = parser.parse_args()
> On 2013/03/12 21:12:33, dhermes wrote:
> > This should go in "if __name__ == '__main__'" and take sys.argv
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode227
> describe.py:227: return string.Template(METHOD_TEMPLATE).substitute(name=name,
> params=params, doc=doc)
> On 2013/03/12 21:12:33, dhermes wrote:
> > Line length
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/describe.py#newcode295
> describe.py:295:
> html.append(string.Template(COLLECTION_LINK).substitute(href=href, name=name))
> On 2013/03/12 21:12:33, dhermes wrote:
> > Line length
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py
> File expand-symlinks.py (right):
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode1
> expand-symlinks.py:1: #!/usr/bin/python2.4
> Renamed to expandsymlinks.py.
> 
> On 2013/03/12 21:12:33, dhermes wrote:
> > This is a module which will never be importable. Is the filename with a '-'
> > character intentional?
> 
> https://codereview.appspot.com/7628044/diff/6001/expand-symlinks.py#newcode42
> expand-symlinks.py:42: FLAGS = parser.parse_args()
> On 2013/03/12 21:12:33, dhermes wrote:
> > This should happen in main.
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py
> File oauth2client/tools.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode50
> oauth2client/tools.py:50: help='Host name to use when running a local web
server
> to '
> On 2013/03/12 21:12:33, dhermes wrote:
> > Bad indent.
> 
> Done.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode52
> oauth2client/tools.py:52: run_parser.add_argument('--noauth_local_webserver',
> action='store_true', default=False,
> On 2013/03/12 21:12:33, dhermes wrote:
> > Line length. Why the n in front of auth_local_webserver?
> 
> To keep the flag name compatible with what came out of gflags, which was
> --noauth_local_webserver to turn the server off.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode56
> oauth2client/tools.py:56: nargs='*', help='Port to use when running a local
web
> server to '
> On 2013/03/12 21:12:33, dhermes wrote:
> > Line length
> 
> Done.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcode59
> oauth2client/tools.py:59: choices=['DEBUG', 'INFO', 'WARNING', 'ERROR',
> 'CRITICAL'],
> On 2013/03/12 21:12:33, dhermes wrote:
> > Bad indent
> 
> Done.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
> oauth2client/tools.py:104: def run(flow, storage, flags, http=None):
> Acknowledged.
> 
> On 2013/03/12 21:12:33, dhermes wrote:
> > IMO it's safer to make flags a required keyword arg than it is to add a
> > positional arg. Up to you.
> 
>
https://codereview.appspot.com/7628044/diff/6001/oauth2client/tools.py#newcod...
> oauth2client/tools.py:227: return """WARNING: Please configure OAuth 2.0
> On 2013/03/12 21:12:33, dhermes wrote:
> > Make this template a top-level constant.
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py
> File samples/plus/plus.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/samples/plus/plus.py#newcode83
> samples/plus/plus.py:83: while (request != None):
> On 2013/03/12 21:12:33, dhermes wrote:
> > You can lose the parens all together. Also, style guide says use "is" or "is
> > not" for None comparisons, not "!=".
> 
> Done.
> 
> https://codereview.appspot.com/7628044/diff/6001/setup.py
> File setup.py (left):
> 
> https://codereview.appspot.com/7628044/diff/6001/setup.py#oldcode64
> setup.py:64: scripts=['bin/enable-app-engine-project'],
> Yes, it's no longer documented as the way to install to App Engine:
> 
>   https://developers.google.com/api-client-library/python/start/installation
> 
> On 2013/03/12 21:12:33, dhermes wrote:
> > Did you mean to axe this?
> 
> https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py
> File tests/test_discovery.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:77: positional_parameters_enforcement =
> POSITIONAL_EXCEPTION
> On 2013/03/12 21:12:33, dhermes wrote:
> > This won't do what you think it's doing. You need to change it on the
module.
> 
> Done.
> 
>
https://codereview.appspot.com/7628044/diff/6001/tests/test_discovery.py#newc...
> tests/test_discovery.py:1035: self.assertEqual(expected,
> simplejson.loads(e.content.decode('utf-8')),
> On 2013/03/12 21:12:33, dhermes wrote:
> > Why this? I'm going to assume this is also part of the Python3 stuff. We
> should
> > not commingle the two sets of changes.
> 
> Done.

[dhermes, 2013-03-18 20:31:01]

Mostly looks good to me (save one comment).

Were you going to update the other samples (like plus) or do that in another
change?

https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
File oauth2client/tools.py (right):

https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
oauth2client/tools.py:45: To make this sample run you will need to populate the
client_secrets.json file
AFAICT these indents were only there to line up with the return. You should
dedent this line and the following ones by two spaces.

[jcgregorio_google, 2013-03-26 17:30:26]

I will update all the rest of the samples in another CL.

I also took another pass at plus.py to simplify it further.

https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
File oauth2client/tools.py (right):

https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
oauth2client/tools.py:45: To make this sample run you will need to populate the
client_secrets.json file
On 2013/03/18 20:31:01, dhermes wrote:
> AFAICT these indents were only there to line up with the return. You should
> dedent this line and the following ones by two spaces.

Done.

[jcgregorio_google, 2013-03-26 17:41:17]

Great, I fat-fingered the upload and overwrote it with httplib2 changes, let me
see if I can revert.

On 2013/03/26 17:30:26, jcgregorio_google wrote:
> I will update all the rest of the samples in another CL.
> 
> I also took another pass at plus.py to simplify it further.
> 
> https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
> File oauth2client/tools.py (right):
> 
>
https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
> oauth2client/tools.py:45: To make this sample run you will need to populate
the
> client_secrets.json file
> On 2013/03/18 20:31:01, dhermes wrote:
> > AFAICT these indents were only there to line up with the return. You should
> > dedent this line and the following ones by two spaces.
> 
> Done.

[jcgregorio_google, 2013-03-26 18:01:31]

OK, fixed.

  -joe

On Tue, Mar 26, 2013 at 1:41 PM,  <jcgregorio@google.com> wrote:
> Great, I fat-fingered the upload and overwrote it with httplib2 changes,
> let me see if I can revert.
>
>
> On 2013/03/26 17:30:26, jcgregorio_google wrote:
>>
>> I will update all the rest of the samples in another CL.
>
>
>> I also took another pass at plus.py to simplify it further.
>
>
>
> https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
>>
>> File oauth2client/tools.py (right):
>
>
>
>
https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
>>
>> oauth2client/tools.py:45: To make this sample run you will need to
>
> populate the
>>
>> client_secrets.json file
>> On 2013/03/18 20:31:01, dhermes wrote:
>> > AFAICT these indents were only there to line up with the return. You
>
> should
>>
>> > dedent this line and the following ones by two spaces.
>
>
>> Done.
>
>
>
>
> https://codereview.appspot.com/7628044/

[dhermes, 2013-03-26 21:35:50]

Sorry, having a hard time figuring out what has changed since Patch Set 7
(the last time I looked AFAICT).

What's more, there seem to be a few patch sets in a permanently broken
state:
https://codereview.appspot.com/7628044/diff2/51001:74002/Makefile
https://codereview.appspot.com/7628044/diff2/51001:74002/README
They both just say "Upload in progress."


On Tue, Mar 26, 2013 at 11:01 AM, Joe Gregorio <jcgregorio@google.com>wrote:

> OK, fixed.
>
>   -joe
>
> On Tue, Mar 26, 2013 at 1:41 PM,  <jcgregorio@google.com> wrote:
> > Great, I fat-fingered the upload and overwrote it with httplib2 changes,
> > let me see if I can revert.
> >
> >
> > On 2013/03/26 17:30:26, jcgregorio_google wrote:
> >>
> >> I will update all the rest of the samples in another CL.
> >
> >
> >> I also took another pass at plus.py to simplify it further.
> >
> >
> >
> > https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
> >>
> >> File oauth2client/tools.py (right):
> >
> >
> >
> >
>
https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
> >>
> >> oauth2client/tools.py:45: To make this sample run you will need to
> >
> > populate the
> >>
> >> client_secrets.json file
> >> On 2013/03/18 20:31:01, dhermes wrote:
> >> > AFAICT these indents were only there to line up with the return. You
> >
> > should
> >>
> >> > dedent this line and the following ones by two spaces.
> >
> >
> >> Done.
> >
> >
> >
> >
> > https://codereview.appspot.com/7628044/
>



-- 
Danny Hermes
Developer Programs Engineer

[joe_bitworking.org, 2013-03-27 17:12:29]

Look at the diffs between 8 and 11. The only files to change are
plus.py and tools.py.

On Tue, Mar 26, 2013 at 5:35 PM, Danny Hermes <dhermes@google.com> wrote:
> Sorry, having a hard time figuring out what has changed since Patch Set 7
> (the last time I looked AFAICT).
>
> What's more, there seem to be a few patch sets in a permanently broken
> state:
> https://codereview.appspot.com/7628044/diff2/51001:74002/Makefile
> https://codereview.appspot.com/7628044/diff2/51001:74002/README
> They both just say "Upload in progress."
>
>
> On Tue, Mar 26, 2013 at 11:01 AM, Joe Gregorio <jcgregorio@google.com>
> wrote:
>>
>> OK, fixed.
>>
>>   -joe
>>
>> On Tue, Mar 26, 2013 at 1:41 PM,  <jcgregorio@google.com> wrote:
>> > Great, I fat-fingered the upload and overwrote it with httplib2 changes,
>> > let me see if I can revert.
>> >
>> >
>> > On 2013/03/26 17:30:26, jcgregorio_google wrote:
>> >>
>> >> I will update all the rest of the samples in another CL.
>> >
>> >
>> >> I also took another pass at plus.py to simplify it further.
>> >
>> >
>> >
>> > https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
>> >>
>> >> File oauth2client/tools.py (right):
>> >
>> >
>> >
>> >
>> >
https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
>> >>
>> >> oauth2client/tools.py:45: To make this sample run you will need to
>> >
>> > populate the
>> >>
>> >> client_secrets.json file
>> >> On 2013/03/18 20:31:01, dhermes wrote:
>> >> > AFAICT these indents were only there to line up with the return. You
>> >
>> > should
>> >>
>> >> > dedent this line and the following ones by two spaces.
>> >
>> >
>> >> Done.
>> >
>> >
>> >
>> >
>> > https://codereview.appspot.com/7628044/
>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer



-- 
Joe Gregorio        http://bitworking.org

[jcgregorio_google, 2013-03-29 15:58:07]

ping

On 2013/03/27 17:12:29, joe_bitworking.org wrote:
> Look at the diffs between 8 and 11. The only files to change are
> plus.py and tools.py.
> 
> On Tue, Mar 26, 2013 at 5:35 PM, Danny Hermes <mailto:dhermes@google.com>
wrote:
> > Sorry, having a hard time figuring out what has changed since Patch Set 7
> > (the last time I looked AFAICT).
> >
> > What's more, there seem to be a few patch sets in a permanently broken
> > state:
> > https://codereview.appspot.com/7628044/diff2/51001:74002/Makefile
> > https://codereview.appspot.com/7628044/diff2/51001:74002/README
> > They both just say "Upload in progress."
> >
> >
> > On Tue, Mar 26, 2013 at 11:01 AM, Joe Gregorio
<mailto:jcgregorio@google.com>
> > wrote:
> >>
> >> OK, fixed.
> >>
> >>   -joe
> >>
> >> On Tue, Mar 26, 2013 at 1:41 PM,  <mailto:jcgregorio@google.com> wrote:
> >> > Great, I fat-fingered the upload and overwrote it with httplib2 changes,
> >> > let me see if I can revert.
> >> >
> >> >
> >> > On 2013/03/26 17:30:26, jcgregorio_google wrote:
> >> >>
> >> >> I will update all the rest of the samples in another CL.
> >> >
> >> >
> >> >> I also took another pass at plus.py to simplify it further.
> >> >
> >> >
> >> >
> >> > https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py
> >> >>
> >> >> File oauth2client/tools.py (right):
> >> >
> >> >
> >> >
> >> >
> >> >
>
https://codereview.appspot.com/7628044/diff/12004/oauth2client/tools.py#newco...
> >> >>
> >> >> oauth2client/tools.py:45: To make this sample run you will need to
> >> >
> >> > populate the
> >> >>
> >> >> client_secrets.json file
> >> >> On 2013/03/18 20:31:01, dhermes wrote:
> >> >> > AFAICT these indents were only there to line up with the return. You
> >> >
> >> > should
> >> >>
> >> >> > dedent this line and the following ones by two spaces.
> >> >
> >> >
> >> >> Done.
> >> >
> >> >
> >> >
> >> >
> >> > https://codereview.appspot.com/7628044/
> >
> >
> >
> >
> > --
> > Danny Hermes
> > Developer Programs Engineer
> 
> 
> 
> -- 
> Joe Gregorio        http://bitworking.org

[dhermes, 2013-03-29 20:15:27]

LGTM

[jcgregorio_google, 2013-03-29 21:11:03]

On 2013/03/29 20:15:27, dhermes wrote:
> LGTM

Committed in
https://code.google.com/p/google-api-python-client/source/detail?r=3116dcdb8b...