Issue 4960063: Added a sample app which uses orgUnit, sites and profiles APIs
|
Created:
12 years, 7 months ago by gunjansharma Modified:
12 years, 7 months ago CC:
gdata-python-client-library-contributors_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 19
Patch Set 2 : Removed some bugs #
Total comments: 6
Patch Set 3 : Updated to accomodate comments and also 80 length line-size #MessagesTotal messages: 8
http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py File samples/apps/org_unit_sites.py (right): http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:18: __author__ = 'GUNJAN SHARMA <gunjansharma@google.com>' s/GUNJAN SHARMA/Gunjan Sharma http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:34: import gdata.contacts.data sort the imports, use pylint to check this file http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:37: ORG_SITE_TITLE = 'Organization Hirarchy' s/Hirarchy/Hierarchy http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:40: #The start of a URI this comment doesn't really help, what is this base uri used for? http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:46: #Header for the new webpages (don't change) don't change? what if I want to change the header and footer? Perhaps you should say what has to be kept to have the application working correctly http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:52: class OrgUnitSites(object): I'd call it OrgUnitAddressBook http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:62: email: [string] The e-mail address of the admin. s/e-mail/email (for consistency with the rest of the file) http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:72: self.sites_client = gdata.sites.client.SitesClient(source=source, domain=domain) line too long http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:88: """Returns the corresponding site_name. explain why you are doing this http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:101: """Returns the corresponding uri to the site. explain the reason to do this http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:138: leave just a single empty line here http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:176: profile: [gdata.contacts.data.ProfileEntry] It is the profile of the object. what is "the object"? is it "the user"? http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:181: new_html = '<h2>Name:</h2><p>%s</p><h2>Gender:</h2><p> %s</p><h2>Address:</h2><p>' % (profile.name.full_name.text,str(profile.gender)) move these string templates into constants http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:217: users = self.org_unit_client.RetrieveAllOrgUsers(self.customer_id) extra whitespace here http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:295: print 'Starting the process. This may take few minutes. Please keep patience.' remove "Please keep patience" http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:301: print 'your site is done!! Cheers \m/' I'd remove the "cheers" :) merge this with the following line, like this: 'Your website is ready, visit it at..." http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:306: """Runs the site making module using a object of OrgUnitSites""" it would be more correct to use "instance" instead of "object" http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:317: # Process options s/Process/Parse http://codereview.appspot.com/4960063/diff/1/samples/apps/org_unit_sites.py#n... samples/apps/org_unit_sites.py:335: choice = raw_input('You have not given us the domain name. Is it %s. (y/n)' % (domain)) This is a question, use a question mark
Sign in to reply to this message.
On 2011/09/12 22:54:31, gunjansharma wrote: Removed the bugs! Done
Sign in to reply to this message.
+ afshar, Ali, Can you review this code? Thanks Claudio On Mon, Sep 12, 2011 at 3:56 PM, <gunjansharma@google.com> wrote: > On 2011/09/12 22:54:31, gunjansharma wrote: > > Removed the bugs! > Done > > > http://codereview.appspot.com/**4960063/<http://codereview.appspot.com/4960063/> >
Sign in to reply to this message.
Some comments of mine from the previous round were not addressed, please check them as well http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.py File samples/apps/org_unit_sites.py (right): http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:127: sit_entry = None s/sit_entry/site_entry http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:132: site_found=True add whitespaces around the = http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:194: new_html = TEMPLATE_USER_HTML % (profile.name.full_name.text,str(profile.gender), address_string, email_string) add a whitespace after the , http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:221: users = self.org_unit_client.RetrieveAllOrgUsers(self.customer_id) there are still two whitespaces after the = http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:305: print 'Your website is ready visit it at: %s' % (self._GetSiteURI(ORG_SITE_TITLE)) add a , after 'ready' http://codereview.appspot.com/4960063/diff/5001/samples/apps/org_unit_sites.p... samples/apps/org_unit_sites.py:309: """Runs the site making module using a object of OrgUnitSites""" OrgUnitSites is now called OrgUnitAddressBook
Sign in to reply to this message.
|
