Issue 5443048: Final file to get reviewed
|
Created:
12 years, 5 months ago by gunjansharma Modified:
12 years, 5 months ago Visibility:
Public. |
Patch Set 1 #
Total comments: 14
Patch Set 2 : resolved comments #
Total comments: 2
Patch Set 3 : Added comment #
Total comments: 2
MessagesTotal messages: 11
http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:17: """Sample illustrating use of new Groups Migration API. replace "new" with "the" http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:53: # The scope for the groups migration API Groups Migration is like a brand, use capital letters http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:100: # Set the logging according to the command-line flag s/flag/flags http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:104: # flow. The Storage object will ensure that if successful the good s/good/valid http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:112: # with our good Credentials. s/good Credentials/valid credentials http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:121: # the OAuth scope is 'https://www.googleapis.com/auth/apps.groups.settings' this is not the correct scope http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:147: response = service.archive().insert(groupId='group1@acdcdasher.com', don't use your real test domain here. Why don't you set a constant called TARGET_GROUP at the top of this file with a comment saying to replace it with a real group id?
Sign in to reply to this message.
http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:17: """Sample illustrating use of new Groups Migration API. On 2011/11/28 18:46:38, Claudio Cherubino wrote: > replace "new" with "the" Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:53: # The scope for the groups migration API On 2011/11/28 18:46:38, Claudio Cherubino wrote: > Groups Migration is like a brand, use capital letters Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:100: # Set the logging according to the command-line flag On 2011/11/28 18:46:38, Claudio Cherubino wrote: > s/flag/flags Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:104: # flow. The Storage object will ensure that if successful the good On 2011/11/28 18:46:38, Claudio Cherubino wrote: > s/good/valid Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:112: # with our good Credentials. On 2011/11/28 18:46:38, Claudio Cherubino wrote: > s/good Credentials/valid credentials Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:121: # the OAuth scope is 'https://www.googleapis.com/auth/apps.groups.settings' On 2011/11/28 18:46:38, Claudio Cherubino wrote: > this is not the correct scope Done. http://codereview.appspot.com/5443048/diff/1/CodeReview/groups_migration.py#n... CodeReview/groups_migration.py:147: response = service.archive().insert(groupId='group1@acdcdasher.com', On 2011/11/28 18:46:38, Claudio Cherubino wrote: > don't use your real test domain here. > > Why don't you set a constant called TARGET_GROUP at the top of this file with a > comment saying to replace it with a real group id? Done.
Sign in to reply to this message.
http://codereview.appspot.com/5443048/diff/5001/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/5001/CodeReview/groups_migration.p... CodeReview/groups_migration.py:84: TARGET_GROUP = 'your_group@your_domain.com' please add a comment to this
Sign in to reply to this message.
http://codereview.appspot.com/5443048/diff/5001/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/5001/CodeReview/groups_migration.p... CodeReview/groups_migration.py:84: TARGET_GROUP = 'your_group@your_domain.com' On 2011/11/28 19:00:59, Claudio Cherubino wrote: > please add a comment to this Done.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
The path name of this file is still incorrect. It should be under the samples/ directory, probably samples/apps/groups_migration/. http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.p... CodeReview/groups_migration.py:120: f = file(os.path.join(os.path.dirname(__file__), Is the groups migration API going to become a public API? Why is the discovery document being loaded from a local file?
Sign in to reply to this message.
The code is not supposed to reside in GData Python client library. The reviewers were automatically added. I never realized it. Have changed it now. http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.py File CodeReview/groups_migration.py (right): http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.p... CodeReview/groups_migration.py:120: f = file(os.path.join(os.path.dirname(__file__), On 2011/11/28 20:45:53, jcgregorio wrote: > Is the groups migration API going to become a public API? Why is the discovery > document being loaded from a local file? This is in TT and the sample is meant for the TTs only. Before we make the API public we will change the code accordingly.
Sign in to reply to this message.
LGTM On 2011/11/29 19:29:01, gunjansharma wrote: > The code is not supposed to reside in GData Python client library. The reviewers > were automatically added. I never realized it. Have changed it now. > > http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.py > File CodeReview/groups_migration.py (right): > > http://codereview.appspot.com/5443048/diff/2003/CodeReview/groups_migration.p... > CodeReview/groups_migration.py:120: f = > file(os.path.join(os.path.dirname(__file__), > On 2011/11/28 20:45:53, jcgregorio wrote: > > Is the groups migration API going to become a public API? Why is the discovery > > document being loaded from a local file? > This is in TT and the sample is meant for the TTs only. Before we make the API > public we will change the code accordingly.
Sign in to reply to this message.
|
