Issue 4867056: Added sample code for the Google Affiliate Network API
|
Created:
13 years ago by namos Modified:
13 years ago CC:
google-api-python-client_googlegroups.com Visibility:
Public. |
Patch Set 1 #Patch Set 2 : Removed unnecessary files. #
Total comments: 10
Patch Set 3 : Small fixes, added print to stdout for returned data #
Total comments: 8
Patch Set 4 : Updated output file options #Patch Set 5 : Fix indent in offers #
MessagesTotal messages: 17
There are a lot of moving parts here. I think test.py and test.html can be dropped. I think I see what you want to do with creds.py, but that could be achieved by just passing the same file name to Storage in both offers.py and events.py. Since these are command line samples just generate plain text to stdout and drop the html formatting to an output file.
Sign in to reply to this message.
The APIs return a large amount of data, so it's not clear that printing the data in plain text to stdout would be of that much use without the html formatting. On 2011/08/17 13:54:28, jcgregorio_google wrote: > There are a lot of moving parts here. > > I think test.py and test.html can be dropped. > > I think I see what you want to do with creds.py, but that could be achieved by > just passing the same file name to Storage in both offers.py and events.py. > > Since these are command line samples just generate plain text to stdout and drop > the html formatting to an output file.
Sign in to reply to this message.
I general, I would consider: 1. removing the html templates, and write prettified JSON to stdout. 2. factoring out the non-application specific code, ie flags and auth so that the application-specific code stands out better. Also one or two minor points. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... File samples/django_sample/gan/ccoffers/offers.py (right): http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:64: Two lines between top-levels. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:70: print e raise-ing the error might be better, or printing something more user-friendly? http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:109: out.write(template.render(context).encode('UTF-8')) I think printing it out nicely to stdout might be nicer. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... File samples/django_sample/gan/events/events.py (right): http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... samples/django_sample/gan/events/events.py:110: params['roleId'] = argv[2] params = { 'role': argv[1], 'roleId': argv[2] } http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... samples/django_sample/gan/events/events.py:144: out = open("output.html", 'w') Again, not sure of the benefit of writing out the result to a file for a demo.
Sign in to reply to this message.
I made the changes you requested. I'm leaning against separating the files to make the app code stand out, since the actual code that calls the api is less than 10 lines. Our focus is to give users an easy base plate to use, which is why we have the html output. So that even if they aren't that familiar with python, they can use these two files to query the API. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... File samples/django_sample/gan/ccoffers/offers.py (right): http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:64: On 2011/08/19 12:47:41, Ali Afshar wrote: > Two lines between top-levels. Done. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:70: print e On 2011/08/19 12:47:41, Ali Afshar wrote: > raise-ing the error might be better, or printing something more user-friendly? Done. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:109: out.write(template.render(context).encode('UTF-8')) On 2011/08/19 12:47:41, Ali Afshar wrote: > I think printing it out nicely to stdout might be nicer. Done. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... File samples/django_sample/gan/events/events.py (right): http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... samples/django_sample/gan/events/events.py:110: params['roleId'] = argv[2] On 2011/08/19 12:47:41, Ali Afshar wrote: > params = { > 'role': argv[1], > 'roleId': argv[2] > } Done. http://codereview.appspot.com/4867056/diff/3001/samples/django_sample/gan/eve... samples/django_sample/gan/events/events.py:144: out = open("output.html", 'w') On 2011/08/19 12:47:41, Ali Afshar wrote: > Again, not sure of the benefit of writing out the result to a file for a demo. Added print to stdout, but would still like to keep the html template. That way if users want to have html output for readability they have a quick boiler-plate solution to start from.
Sign in to reply to this message.
Move samples to either /samples/searchforshopping/gan or /samples/gan/. http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... File samples/django_sample/gan/ccoffers/offers.py (right): http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:55: gflags.DEFINE_string('filename', 'offers.dat', Make the default file name empty and only write the output to file if a name is provided. http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: usage(argv) make this two lines http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): add back when advertiser is repeated Leading space http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:117: print json.dumps(list, sort_keys=True, indent=4) Use FLAGS.filename to choose between either writing output to stdout, or writing to a file.
Sign in to reply to this message.
Sorry about the delay on this. We've been resolving some issues with getting the API visible and working in prod. I have the updates ready, but I'm not able to upload them. Am I still a contributor? Because the upload-diffs.py file keeps telling me I have an invalid username or password. On 2011/08/19 15:36:16, jcgregorio_google wrote: > Move samples to either /samples/searchforshopping/gan or /samples/gan/. > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > File samples/django_sample/gan/ccoffers/offers.py (right): > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:55: > gflags.DEFINE_string('filename', 'offers.dat', > Make the default file name empty and only write the output to file if a name is > provided. > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: usage(argv) > make this two lines > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): add back > when advertiser is repeated > Leading space > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:117: print json.dumps(list, > sort_keys=True, indent=4) > Use FLAGS.filename to choose between either writing output to stdout, or writing > to a file.
Sign in to reply to this message.
Thanks for the tip. I completely forgot about the app specific passwords. </personalmessage> http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... File samples/django_sample/gan/ccoffers/offers.py (right): http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:55: gflags.DEFINE_string('filename', 'offers.dat', On 2011/08/19 15:36:16, jcgregorio_google wrote: > Make the default file name empty and only write the output to file if a name is > provided. Done. http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: usage(argv) On 2011/08/19 15:36:16, jcgregorio_google wrote: > make this two lines Done. http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): add back when advertiser is repeated On 2011/08/19 15:36:16, jcgregorio_google wrote: > Leading space Done. http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... samples/django_sample/gan/ccoffers/offers.py:117: print json.dumps(list, sort_keys=True, indent=4) On 2011/08/19 15:36:16, jcgregorio_google wrote: > Use FLAGS.filename to choose between either writing output to stdout, or writing > to a file. filename was for storing credentials. This is now changed to credentials_filename, and I added an additional flag for setting the output mode.
Sign in to reply to this message.
Are you sure, I'm not seeing any changes on codereview.appspot.com. Did you supply "--issue 4867056" on the command-line to update the existing issue? On Fri, Sep 2, 2011 at 11:02 AM, <namos@google.com> wrote: > Thanks for the tip. I completely forgot about the app specific > passwords. > </personalmessage> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > File samples/django_sample/gan/ccoffers/offers.py (right): > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:55: > gflags.DEFINE_string('filename', 'offers.dat', > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> Make the default file name empty and only write the output to file if > > a name is >> >> provided. > > Done. > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: > usage(argv) > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> make this two lines > > Done. > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): add > back when advertiser is repeated > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> Leading space > > Done. > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > samples/django_sample/gan/ccoffers/offers.py:117: print json.dumps(list, > sort_keys=True, indent=4) > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> Use FLAGS.filename to choose between either writing output to stdout, > > or writing >> >> to a file. > > filename was for storing credentials. This is now changed to > credentials_filename, and I added an additional flag for setting the > output mode. > > http://codereview.appspot.com/4867056/ >
Sign in to reply to this message.
I did. Do you see patch set 4? I moved the files to the new dir that you suggested, so they don't have diffs. On 2011/09/02 15:07:12, jcgregorio_google wrote: > Are you sure, I'm not seeing any changes on http://codereview.appspot.com. Did > you supply "--issue 4867056" on the command-line to update the existing issue? > > On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> wrote: > > Thanks for the tip. I completely forgot about the app specific > > passwords. > > </personalmessage> > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > File samples/django_sample/gan/ccoffers/offers.py (right): > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > samples/django_sample/gan/ccoffers/offers.py:55: > > gflags.DEFINE_string('filename', 'offers.dat', > > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> Make the default file name empty and only write the output to file if > > > > a name is > >> > >> provided. > > > > Done. > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: > > usage(argv) > > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> make this two lines > > > > Done. > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): add > > back when advertiser is repeated > > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> Leading space > > > > Done. > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > samples/django_sample/gan/ccoffers/offers.py:117: print json.dumps(list, > > sort_keys=True, indent=4) > > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> Use FLAGS.filename to choose between either writing output to stdout, > > > > or writing > >> > >> to a file. > > > > filename was for storing credentials. This is now changed to > > credentials_filename, and I added an additional flag for setting the > > output mode. > > > > http://codereview.appspot.com/4867056/ > >
Sign in to reply to this message.
Yes, I see it, and it doesn't contain the changes we talked about. For example, the indenting hasn't been fixed near the "TODO(leadpipe)" in: http://codereview.appspot.com/4867056/patch/16001/17001 -joe On Fri, Sep 2, 2011 at 11:10 AM, <namos@google.com> wrote: > I did. Do you see patch set 4? I moved the files to the new dir that you > suggested, so they don't have diffs. > > On 2011/09/02 15:07:12, jcgregorio_google wrote: >> >> Are you sure, I'm not seeing any changes on > > http://codereview.appspot.com. Did >> >> you supply "--issue 4867056" on the command-line to update the > > existing issue? > >> On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> wrote: >> > Thanks for the tip. I completely forgot about the app specific >> > passwords. >> > </personalmessage> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > samples/django_sample/gan/ccoffers/offers.py:55: >> > gflags.DEFINE_string('filename', 'offers.dat', >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> Make the default file name empty and only write the output to file > > if >> >> > >> > a name is >> >> >> >> provided. >> > >> > Done. >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: >> > usage(argv) >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> make this two lines >> > >> > Done. >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): > > add >> >> > back when advertiser is repeated >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> Leading space >> > >> > Done. >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print > > json.dumps(list, >> >> > sort_keys=True, indent=4) >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> Use FLAGS.filename to choose between either writing output to > > stdout, >> >> > >> > or writing >> >> >> >> to a file. >> > >> > filename was for storing credentials. This is now changed to >> > credentials_filename, and I added an additional flag for setting the >> > output mode. >> > >> > http://codereview.appspot.com/4867056/ >> > > > > > http://codereview.appspot.com/4867056/ >
Sign in to reply to this message.
Is that the only thing that is missing? I must have misunderstood what that comment meant. I thought it meant add a space between # and "advertiser..." on the next line. Should the comments be indented only two spaces? On 2011/09/02 15:14:23, jcgregorio_google wrote: > Yes, I see it, and it doesn't contain the changes we talked about. For > example, the indenting > hasn't been fixed near the "TODO(leadpipe)" in: > > http://codereview.appspot.com/4867056/patch/16001/17001 > > -joe > > On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> wrote: > > I did. Do you see patch set 4? I moved the files to the new dir that you > > suggested, so they don't have diffs. > > > > On 2011/09/02 15:07:12, jcgregorio_google wrote: > >> > >> Are you sure, I'm not seeing any changes on > > > > http://codereview.appspot.com. Did > >> > >> you supply "--issue 4867056" on the command-line to update the > > > > existing issue? > > > >> On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> wrote: > >> > Thanks for the tip. I completely forgot about the app specific > >> > passwords. > >> > </personalmessage> > >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > File samples/django_sample/gan/ccoffers/offers.py (right): > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > samples/django_sample/gan/ccoffers/offers.py:55: > >> > gflags.DEFINE_string('filename', 'offers.dat', > >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> > >> >> Make the default file name empty and only write the output to file > > > > if > >> > >> > > >> > a name is > >> >> > >> >> provided. > >> > > >> > Done. > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != 2: > >> > usage(argv) > >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> > >> >> make this two lines > >> > > >> > Done. > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > samples/django_sample/gan/ccoffers/offers.py:100: # TODO(leadpipe): > > > > add > >> > >> > back when advertiser is repeated > >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> > >> >> Leading space > >> > > >> > Done. > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > samples/django_sample/gan/ccoffers/offers.py:117: print > > > > json.dumps(list, > >> > >> > sort_keys=True, indent=4) > >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> > >> >> Use FLAGS.filename to choose between either writing output to > > > > stdout, > >> > >> > > >> > or writing > >> >> > >> >> to a file. > >> > > >> > filename was for storing credentials. This is now changed to > >> > credentials_filename, and I added an additional flag for setting the > >> > output mode. > >> > > >> > http://codereview.appspot.com/4867056/ > >> > > > > > > > > > http://codereview.appspot.com/4867056/ > >
Sign in to reply to this message.
On Fri, Sep 2, 2011 at 11:30 AM, <namos@google.com> wrote: > Is that the only thing that is missing? I must have misunderstood what > that comment meant. I thought it meant add a space between # and > "advertiser..." on the next line. Should the comments be indented only > two spaces? Yes, if you align the parameters then you exceed 80 chars. Sorry for the confusion. -joe > > On 2011/09/02 15:14:23, jcgregorio_google wrote: >> >> Yes, I see it, and it doesn't contain the changes we talked about. For >> example, the indenting >> hasn't been fixed near the "TODO(leadpipe)" in: > >> http://codereview.appspot.com/4867056/patch/16001/17001 > >> -joe > >> On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> wrote: >> > I did. Do you see patch set 4? I moved the files to the new dir that > > you >> >> > suggested, so they don't have diffs. >> > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: >> >> >> >> Are you sure, I'm not seeing any changes on >> > >> > http://codereview.appspot.com. Did >> >> >> >> you supply "--issue 4867056" on the command-line to update > > the >> >> > >> > existing issue? >> > >> >> On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> > > wrote: >> >> >> > Thanks for the tip. I completely forgot about the app specific >> >> > passwords. >> >> > </personalmessage> >> >> > >> >> > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): >> >> > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > samples/django_sample/gan/ccoffers/offers.py:55: >> >> > gflags.DEFINE_string('filename', 'offers.dat', >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> >> >> Make the default file name empty and only write the output to > > file >> >> > >> > if >> >> >> >> > >> >> > a name is >> >> >> >> >> >> provided. >> >> > >> >> > Done. >> >> > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != > > 2: >> >> >> > usage(argv) >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> >> >> make this two lines >> >> > >> >> > Done. >> >> > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # > > TODO(leadpipe): >> >> > >> > add >> >> >> >> > back when advertiser is repeated >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> >> >> Leading space >> >> > >> >> > Done. >> >> > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print >> > >> > json.dumps(list, >> >> >> >> > sort_keys=True, indent=4) >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> >> >> >> >> Use FLAGS.filename to choose between either writing output to >> > >> > stdout, >> >> >> >> > >> >> > or writing >> >> >> >> >> >> to a file. >> >> > >> >> > filename was for storing credentials. This is now changed to >> >> > credentials_filename, and I added an additional flag for setting > > the >> >> >> > output mode. >> >> > >> >> > http://codereview.appspot.com/4867056/ >> >> > >> > >> > >> > >> > http://codereview.appspot.com/4867056/ >> > > > > > http://codereview.appspot.com/4867056/ >
Sign in to reply to this message.
Alright, should be fixed now. Thanks, Ryan On 2011/09/02 15:47:58, jcgregorio_google wrote: > On Fri, Sep 2, 2011 at 11:30 AM, <mailto:namos@google.com> wrote: > > Is that the only thing that is missing? I must have misunderstood what > > that comment meant. I thought it meant add a space between # and > > "advertiser..." on the next line. Should the comments be indented only > > two spaces? > > Yes, if you align the parameters then you exceed 80 chars. > > Sorry for the confusion. > -joe > > > > > On 2011/09/02 15:14:23, jcgregorio_google wrote: > >> > >> Yes, I see it, and it doesn't contain the changes we talked about. For > >> example, the indenting > >> hasn't been fixed near the "TODO(leadpipe)" in: > > > >> http://codereview.appspot.com/4867056/patch/16001/17001 > > > >> -joe > > > >> On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> wrote: > >> > I did. Do you see patch set 4? I moved the files to the new dir that > > > > you > >> > >> > suggested, so they don't have diffs. > >> > > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: > >> >> > >> >> Are you sure, I'm not seeing any changes on > >> > > >> > http://codereview.appspot.com. Did > >> >> > >> >> you supply "--issue 4867056" on the command-line to update > > > > the > >> > >> > > >> > existing issue? > >> > > >> >> On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> > > > > wrote: > >> > >> >> > Thanks for the tip. I completely forgot about the app specific > >> >> > passwords. > >> >> > </personalmessage> > >> >> > > >> >> > > >> >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> >> > >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): > >> >> > > >> >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:55: > >> >> > gflags.DEFINE_string('filename', 'offers.dat', > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> >> > >> >> >> Make the default file name empty and only write the output to > > > > file > >> > >> > > >> > if > >> >> > >> >> > > >> >> > a name is > >> >> >> > >> >> >> provided. > >> >> > > >> >> > Done. > >> >> > > >> >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != > > > > 2: > >> > >> >> > usage(argv) > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> >> > >> >> >> make this two lines > >> >> > > >> >> > Done. > >> >> > > >> >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # > > > > TODO(leadpipe): > >> > >> > > >> > add > >> >> > >> >> > back when advertiser is repeated > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> >> > >> >> >> Leading space > >> >> > > >> >> > Done. > >> >> > > >> >> > > >> > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print > >> > > >> > json.dumps(list, > >> >> > >> >> > sort_keys=True, indent=4) > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> >> >> > >> >> >> Use FLAGS.filename to choose between either writing output to > >> > > >> > stdout, > >> >> > >> >> > > >> >> > or writing > >> >> >> > >> >> >> to a file. > >> >> > > >> >> > filename was for storing credentials. This is now changed to > >> >> > credentials_filename, and I added an additional flag for setting > > > > the > >> > >> >> > output mode. > >> >> > > >> >> > http://codereview.appspot.com/4867056/ > >> >> > > >> > > >> > > >> > > >> > http://codereview.appspot.com/4867056/ > >> > > > > > > > > > http://codereview.appspot.com/4867056/ > >
Sign in to reply to this message.
What's the status on this? Is there anything else I need to change? Thanks, Ryan On 2011/09/02 15:57:38, namos wrote: > Alright, should be fixed now. > Thanks, > Ryan > > On 2011/09/02 15:47:58, jcgregorio_google wrote: > > On Fri, Sep 2, 2011 at 11:30 AM, <mailto:namos@google.com> wrote: > > > Is that the only thing that is missing? I must have misunderstood what > > > that comment meant. I thought it meant add a space between # and > > > "advertiser..." on the next line. Should the comments be indented only > > > two spaces? > > > > Yes, if you align the parameters then you exceed 80 chars. > > > > Sorry for the confusion. > > -joe > > > > > > > > On 2011/09/02 15:14:23, jcgregorio_google wrote: > > >> > > >> Yes, I see it, and it doesn't contain the changes we talked about. For > > >> example, the indenting > > >> hasn't been fixed near the "TODO(leadpipe)" in: > > > > > >> http://codereview.appspot.com/4867056/patch/16001/17001 > > > > > >> -joe > > > > > >> On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> wrote: > > >> > I did. Do you see patch set 4? I moved the files to the new dir that > > > > > > you > > >> > > >> > suggested, so they don't have diffs. > > >> > > > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: > > >> >> > > >> >> Are you sure, I'm not seeing any changes on > > >> > > > >> > http://codereview.appspot.com. Did > > >> >> > > >> >> you supply "--issue 4867056" on the command-line to update > > > > > > the > > >> > > >> > > > >> > existing issue? > > >> > > > >> >> On Fri, Sep 2, 2011 at 11:02 AM, <mailto:namos@google.com> > > > > > > wrote: > > >> > > >> >> > Thanks for the tip. I completely forgot about the app specific > > >> >> > passwords. > > >> >> > </personalmessage> > > >> >> > > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > >> > > >> >> > > >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > >> > > >> >> > > >> >> > samples/django_sample/gan/ccoffers/offers.py:55: > > >> >> > gflags.DEFINE_string('filename', 'offers.dat', > > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > > >> >> >> > > >> >> >> Make the default file name empty and only write the output to > > > > > > file > > >> > > >> > > > >> > if > > >> >> > > >> >> > > > >> >> > a name is > > >> >> >> > > >> >> >> provided. > > >> >> > > > >> >> > Done. > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > >> > > >> >> > > >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if len(argv) != > > > > > > 2: > > >> > > >> >> > usage(argv) > > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > > >> >> >> > > >> >> >> make this two lines > > >> >> > > > >> >> > Done. > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > >> > > >> >> > > >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # > > > > > > TODO(leadpipe): > > >> > > >> > > > >> > add > > >> >> > > >> >> > back when advertiser is repeated > > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > > >> >> >> > > >> >> >> Leading space > > >> >> > > > >> >> > Done. > > >> >> > > > >> >> > > > >> > > > >> > > > > > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > > >> > > >> >> > > >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print > > >> > > > >> > json.dumps(list, > > >> >> > > >> >> > sort_keys=True, indent=4) > > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > > >> >> >> > > >> >> >> Use FLAGS.filename to choose between either writing output to > > >> > > > >> > stdout, > > >> >> > > >> >> > > > >> >> > or writing > > >> >> >> > > >> >> >> to a file. > > >> >> > > > >> >> > filename was for storing credentials. This is now changed to > > >> >> > credentials_filename, and I added an additional flag for setting > > > > > > the > > >> > > >> >> > output mode. > > >> >> > > > >> >> > http://codereview.appspot.com/4867056/ > > >> >> > > > >> > > > >> > > > >> > > > >> > http://codereview.appspot.com/4867056/ > > >> > > > > > > > > > > > > > http://codereview.appspot.com/4867056/ > > >
Sign in to reply to this message.
Sorry for the delay, just committed: http://code.google.com/p/google-api-python-client/source/detail?r=7726fb4cb17... Can you write up a separate CL that adds a README to the sample to let people know they need Django installed to run the sample? Thanks, -joe On Thu, Sep 8, 2011 at 4:02 PM, <namos@google.com> wrote: > What's the status on this? Is there anything else I need to change? > > Thanks, > Ryan > On 2011/09/02 15:57:38, namos wrote: >> >> Alright, should be fixed now. >> Thanks, >> Ryan > >> On 2011/09/02 15:47:58, jcgregorio_google wrote: >> > On Fri, Sep 2, 2011 at 11:30 AM, <mailto:namos@google.com> wrote: >> > > Is that the only thing that is missing? I must have misunderstood > > what >> >> > > that comment meant. I thought it meant add a space between # and >> > > "advertiser..." on the next line. Should the comments be indented > > only >> >> > > two spaces? >> > >> > Yes, if you align the parameters then you exceed 80 chars. >> > >> > Sorry for the confusion. >> > -joe >> > >> > > >> > > On 2011/09/02 15:14:23, jcgregorio_google wrote: >> > >> >> > >> Yes, I see it, and it doesn't contain the changes we talked > > about. For >> >> > >> example, the indenting >> > >> hasn't been fixed near the "TODO(leadpipe)" in: >> > > >> > >> http://codereview.appspot.com/4867056/patch/16001/17001 >> > > >> > >> -joe >> > > >> > >> On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> > > wrote: >> >> > >> > I did. Do you see patch set 4? I moved the files to the new dir > > that >> >> > > >> > > you >> > >> >> > >> > suggested, so they don't have diffs. >> > >> > >> > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: >> > >> >> >> > >> >> Are you sure, I'm not seeing any changes on >> > >> > >> > >> > http://codereview.appspot.com. Did >> > >> >> >> > >> >> you supply "--issue 4867056" on the command-line to > > update >> >> > > >> > > the >> > >> >> > >> > >> > >> > existing issue? >> > >> > >> > >> >> On Fri, Sep 2, 2011 at 11:02 AM, > > <mailto:namos@google.com> >> >> > > >> > > wrote: >> > >> >> > >> >> > Thanks for the tip. I completely forgot about the app > > specific >> >> > >> >> > passwords. >> > >> >> > </personalmessage> >> > >> >> > >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > >> >> > >> >> >> > >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > >> >> > >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:55: >> > >> >> > gflags.DEFINE_string('filename', 'offers.dat', >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> > >> >> >> >> > >> >> >> Make the default file name empty and only write the output > > to >> >> > > >> > > file >> > >> >> > >> > >> > >> > if >> > >> >> >> > >> >> > >> > >> >> > a name is >> > >> >> >> >> > >> >> >> provided. >> > >> >> > >> > >> >> > Done. >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > >> >> > >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if > > len(argv) != >> >> > > >> > > 2: >> > >> >> > >> >> > usage(argv) >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> > >> >> >> >> > >> >> >> make this two lines >> > >> >> > >> > >> >> > Done. >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > >> >> > >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # >> > > >> > > TODO(leadpipe): >> > >> >> > >> > >> > >> > add >> > >> >> >> > >> >> > back when advertiser is repeated >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> > >> >> >> >> > >> >> >> Leading space >> > >> >> > >> > >> >> > Done. >> > >> >> > >> > >> >> > >> > >> > >> > >> > >> > > >> > > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> > >> >> > >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print >> > >> > >> > >> > json.dumps(list, >> > >> >> >> > >> >> > sort_keys=True, indent=4) >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> > >> >> >> >> > >> >> >> Use FLAGS.filename to choose between either writing output > > to >> >> > >> > >> > >> > stdout, >> > >> >> >> > >> >> > >> > >> >> > or writing >> > >> >> >> >> > >> >> >> to a file. >> > >> >> > >> > >> >> > filename was for storing credentials. This is now changed to >> > >> >> > credentials_filename, and I added an additional flag for > > setting >> >> > > >> > > the >> > >> >> > >> >> > output mode. >> > >> >> > >> > >> >> > http://codereview.appspot.com/4867056/ >> > >> >> > >> > >> > >> > >> > >> > >> > >> > >> > http://codereview.appspot.com/4867056/ >> > >> > >> > > >> > > >> > > >> > > http://codereview.appspot.com/4867056/ >> > > > > > > http://codereview.appspot.com/4867056/ >
Sign in to reply to this message.
Is it sufficient for it to mention that you need django installed in the codesite google affiliate network api docs? Or do I need a readme as well? On 2011/09/08 20:13:59, jcgregorio_google wrote: > Sorry for the delay, just committed: > http://code.google.com/p/google-api-python-client/source/detail?r=7726fb4cb17... > > Can you write up a separate CL that adds a README to the sample to let > people know they need Django installed to run the sample? > > Thanks, > -joe > > On Thu, Sep 8, 2011 at 4:02 PM, <mailto:namos@google.com> wrote: > > What's the status on this? Is there anything else I need to change? > > > > Thanks, > > Ryan > > On 2011/09/02 15:57:38, namos wrote: > >> > >> Alright, should be fixed now. > >> Thanks, > >> Ryan > > > >> On 2011/09/02 15:47:58, jcgregorio_google wrote: > >> > On Fri, Sep 2, 2011 at 11:30 AM, <mailto:namos@google.com> wrote: > >> > > Is that the only thing that is missing? I must have misunderstood > > > > what > >> > >> > > that comment meant. I thought it meant add a space between # and > >> > > "advertiser..." on the next line. Should the comments be indented > > > > only > >> > >> > > two spaces? > >> > > >> > Yes, if you align the parameters then you exceed 80 chars. > >> > > >> > Sorry for the confusion. > >> > -joe > >> > > >> > > > >> > > On 2011/09/02 15:14:23, jcgregorio_google wrote: > >> > >> > >> > >> Yes, I see it, and it doesn't contain the changes we talked > > > > about. For > >> > >> > >> example, the indenting > >> > >> hasn't been fixed near the "TODO(leadpipe)" in: > >> > > > >> > >> http://codereview.appspot.com/4867056/patch/16001/17001 > >> > > > >> > >> -joe > >> > > > >> > >> On Fri, Sep 2, 2011 at 11:10 AM, <mailto:namos@google.com> > > > > wrote: > >> > >> > >> > I did. Do you see patch set 4? I moved the files to the new dir > > > > that > >> > >> > > > >> > > you > >> > >> > >> > >> > suggested, so they don't have diffs. > >> > >> > > >> > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: > >> > >> >> > >> > >> >> Are you sure, I'm not seeing any changes on > >> > >> > > >> > >> > http://codereview.appspot.com. Did > >> > >> >> > >> > >> >> you supply "--issue 4867056" on the command-line to > > > > update > >> > >> > > > >> > > the > >> > >> > >> > >> > > >> > >> > existing issue? > >> > >> > > >> > >> >> On Fri, Sep 2, 2011 at 11:02 AM, > > > > <mailto:namos@google.com> > >> > >> > > > >> > > wrote: > >> > >> > >> > >> >> > Thanks for the tip. I completely forgot about the app > > > > specific > >> > >> > >> >> > passwords. > >> > >> >> > </personalmessage> > >> > >> >> > > >> > >> >> > > >> > >> >> > > >> > >> > > >> > >> > > >> > > > >> > > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > >> > >> > >> >> > >> > >> >> > File samples/django_sample/gan/ccoffers/offers.py (right): > >> > >> >> > > >> > >> >> > > >> > >> > > >> > >> > > >> > > > >> > > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > >> > >> > >> >> > >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:55: > >> > >> >> > gflags.DEFINE_string('filename', 'offers.dat', > >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> >> >> > >> > >> >> >> Make the default file name empty and only write the output > > > > to > >> > >> > > > >> > > file > >> > >> > >> > >> > > >> > >> > if > >> > >> >> > >> > >> >> > > >> > >> >> > a name is > >> > >> >> >> > >> > >> >> >> provided. > >> > >> >> > > >> > >> >> > Done. > >> > >> >> > > >> > >> >> > > >> > >> > > >> > >> > > >> > > > >> > > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > >> > >> > >> >> > >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if > > > > len(argv) != > >> > >> > > > >> > > 2: > >> > >> > >> > >> >> > usage(argv) > >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> >> >> > >> > >> >> >> make this two lines > >> > >> >> > > >> > >> >> > Done. > >> > >> >> > > >> > >> >> > > >> > >> > > >> > >> > > >> > > > >> > > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > >> > >> > >> >> > >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # > >> > > > >> > > TODO(leadpipe): > >> > >> > >> > >> > > >> > >> > add > >> > >> >> > >> > >> >> > back when advertiser is repeated > >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> >> >> > >> > >> >> >> Leading space > >> > >> >> > > >> > >> >> > Done. > >> > >> >> > > >> > >> >> > > >> > >> > > >> > >> > > >> > > > >> > > > >> > > > > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... > >> > >> > >> > >> > >> >> > >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print > >> > >> > > >> > >> > json.dumps(list, > >> > >> >> > >> > >> >> > sort_keys=True, indent=4) > >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: > >> > >> >> >> > >> > >> >> >> Use FLAGS.filename to choose between either writing output > > > > to > >> > >> > >> > > >> > >> > stdout, > >> > >> >> > >> > >> >> > > >> > >> >> > or writing > >> > >> >> >> > >> > >> >> >> to a file. > >> > >> >> > > >> > >> >> > filename was for storing credentials. This is now changed to > >> > >> >> > credentials_filename, and I added an additional flag for > > > > setting > >> > >> > > > >> > > the > >> > >> > >> > >> >> > output mode. > >> > >> >> > > >> > >> >> > http://codereview.appspot.com/4867056/ > >> > >> >> > > >> > >> > > >> > >> > > >> > >> > > >> > >> > http://codereview.appspot.com/4867056/ > >> > >> > > >> > > > >> > > > >> > > > >> > > http://codereview.appspot.com/4867056/ > >> > > > > > > > > > > http://codereview.appspot.com/4867056/ > >
Sign in to reply to this message.
I'd like it in both as you can't be sure a user would hit the sample first and the docs second. -joe On Thu, Sep 8, 2011 at 4:55 PM, <namos@google.com> wrote: > Is it sufficient for it to mention that you need django installed in the > codesite google affiliate network api docs? Or do I need a readme as > well? > On 2011/09/08 20:13:59, jcgregorio_google wrote: >> >> Sorry for the delay, just committed: > > http://code.google.com/p/google-api-python-client/source/detail?r=7726fb4cb17... > >> Can you write up a separate CL that adds a README to the sample to let >> people know they need Django installed to run the sample? > >> Thanks, >> -joe > >> On Thu, Sep 8, 2011 at 4:02 PM, <mailto:namos@google.com> wrote: >> > What's the status on this? Is there anything else I need to change? >> > >> > Thanks, >> > Ryan >> > On 2011/09/02 15:57:38, namos wrote: >> >> >> >> Alright, should be fixed now. >> >> Thanks, >> >> Ryan >> > >> >> On 2011/09/02 15:47:58, jcgregorio_google wrote: >> >> > On Fri, Sep 2, 2011 at 11:30 AM, <mailto:namos@google.com> > > wrote: >> >> >> > > Is that the only thing that is missing? I must have > > misunderstood >> >> > >> > what >> >> >> >> > > that comment meant. I thought it meant add a space between # > > and >> >> >> > > "advertiser..." on the next line. Should the comments be > > indented >> >> > >> > only >> >> >> >> > > two spaces? >> >> > >> >> > Yes, if you align the parameters then you exceed 80 chars. >> >> > >> >> > Sorry for the confusion. >> >> > -joe >> >> > >> >> > > >> >> > > On 2011/09/02 15:14:23, jcgregorio_google wrote: >> >> > >> >> >> > >> Yes, I see it, and it doesn't contain the changes we talked >> > >> > about. For >> >> >> >> > >> example, the indenting >> >> > >> hasn't been fixed near the "TODO(leadpipe)" in: >> >> > > >> >> > >> http://codereview.appspot.com/4867056/patch/16001/17001 >> >> > > >> >> > >> -joe >> >> > > >> >> > >> On Fri, Sep 2, 2011 at 11:10 AM, > > <mailto:namos@google.com> >> >> > >> > wrote: >> >> >> >> > >> > I did. Do you see patch set 4? I moved the files to the new > > dir >> >> > >> > that >> >> >> >> > > >> >> > > you >> >> > >> >> >> > >> > suggested, so they don't have diffs. >> >> > >> > >> >> > >> > On 2011/09/02 15:07:12, jcgregorio_google wrote: >> >> > >> >> >> >> > >> >> Are you sure, I'm not seeing any changes on >> >> > >> > >> >> > >> > http://codereview.appspot.com. Did >> >> > >> >> >> >> > >> >> you supply "--issue 4867056" on the command-line to >> > >> > update >> >> >> >> > > >> >> > > the >> >> > >> >> >> > >> > >> >> > >> > existing issue? >> >> > >> > >> >> > >> >> On Fri, Sep 2, 2011 at 11:02 AM, >> > >> > <mailto:namos@google.com> >> >> >> >> > > >> >> > > wrote: >> >> > >> >> >> > >> >> > Thanks for the tip. I completely forgot about the app >> > >> > specific >> >> >> >> > >> >> > passwords. >> >> > >> >> > </personalmessage> >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > > >> >> > > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > >> >> >> > >> >> >> >> > >> >> > File samples/django_sample/gan/ccoffers/offers.py > > (right): >> >> >> > >> >> > >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > > >> >> > > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > >> >> >> > >> >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:55: >> >> > >> >> > gflags.DEFINE_string('filename', 'offers.dat', >> >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> > >> >> >> >> >> > >> >> >> Make the default file name empty and only write the > > output >> >> > >> > to >> >> >> >> > > >> >> > > file >> >> > >> >> >> > >> > >> >> > >> > if >> >> > >> >> >> >> > >> >> > >> >> > >> >> > a name is >> >> > >> >> >> >> >> > >> >> >> provided. >> >> > >> >> > >> >> > >> >> > Done. >> >> > >> >> > >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > > >> >> > > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > >> >> >> > >> >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:75: if >> > >> > len(argv) != >> >> >> >> > > >> >> > > 2: >> >> > >> >> >> > >> >> > usage(argv) >> >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> > >> >> >> >> >> > >> >> >> make this two lines >> >> > >> >> > >> >> > >> >> > Done. >> >> > >> >> > >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > > >> >> > > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > >> >> >> > >> >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:100: # >> >> > > >> >> > > TODO(leadpipe): >> >> > >> >> >> > >> > >> >> > >> > add >> >> > >> >> >> >> > >> >> > back when advertiser is repeated >> >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> > >> >> >> >> >> > >> >> >> Leading space >> >> > >> >> > >> >> > >> >> > Done. >> >> > >> >> > >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > > >> >> > > >> >> > >> > >> > > > http://codereview.appspot.com/4867056/diff/9001/samples/django_sample/gan/cco... >> >> >> >> >> > >> >> >> > >> >> >> >> > >> >> > samples/django_sample/gan/ccoffers/offers.py:117: print >> >> > >> > >> >> > >> > json.dumps(list, >> >> > >> >> >> >> > >> >> > sort_keys=True, indent=4) >> >> > >> >> > On 2011/08/19 15:36:16, jcgregorio_google wrote: >> >> > >> >> >> >> >> > >> >> >> Use FLAGS.filename to choose between either writing > > output >> >> > >> > to >> >> >> >> > >> > >> >> > >> > stdout, >> >> > >> >> >> >> > >> >> > >> >> > >> >> > or writing >> >> > >> >> >> >> >> > >> >> >> to a file. >> >> > >> >> > >> >> > >> >> > filename was for storing credentials. This is now changed > > to >> >> >> > >> >> > credentials_filename, and I added an additional flag for >> > >> > setting >> >> >> >> > > >> >> > > the >> >> > >> >> >> > >> >> > output mode. >> >> > >> >> > >> >> > >> >> > http://codereview.appspot.com/4867056/ >> >> > >> >> > >> >> > >> > >> >> > >> > >> >> > >> > >> >> > >> > http://codereview.appspot.com/4867056/ >> >> > >> > >> >> > > >> >> > > >> >> > > >> >> > > http://codereview.appspot.com/4867056/ >> >> > > >> > >> > >> > >> > http://codereview.appspot.com/4867056/ >> > > > > > http://codereview.appspot.com/4867056/ > -- Joe Gregorio http://bitworking.org
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
