Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Issue 11316097: google_apis: Rename and simplify GenerateGetDocumentsURL() (Closed)

Created:
8 years, 1 month ago by satorux1
Modified:
8 years, 1 month ago
Reviewers:
hashimoto
CC:
chromium-reviews, achuith+watch_chromium.org
Visibility:
Public.

Description

google_apis: Rename and simplify GenerateGetDocumentsURL() Rename GenerateGetDocumentsURL() to GenerateDocumentListUrl(), and simplify the code per hashimoto's suggestion. Thanks to the unit test added in the previous patch, we were able to simplify the code safely. :) BUG=161718 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=168747

Patch Set 1 #

Patch Set 2 : polish #

Total comments: 2

Patch Set 3 : address comments #

Total comments: 2

Patch Set 4 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -50 lines) Patch
M chrome/browser/google_apis/gdata_wapi_operations.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/google_apis/gdata_wapi_url_util.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/google_apis/gdata_wapi_url_util.cc View 1 2 3 3 chunks +20 lines, -41 lines 0 comments Download
M chrome/browser/google_apis/gdata_wapi_url_util_unittest.cc View 1 6 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
satorux1
Your suggestion was great!
8 years, 1 month ago (2012-11-20 05:26:06 UTC) #1
satorux1
Please put this on hold. I'm making a bit more cleanup
8 years, 1 month ago (2012-11-20 05:57:42 UTC) #2
hashimoto
lgtm
8 years, 1 month ago (2012-11-20 05:58:40 UTC) #3
hashimoto
On 2012/11/20 05:58:40, hashimoto wrote: > lgtm oops, please forget this comment.
8 years, 1 month ago (2012-11-20 05:59:35 UTC) #4
satorux1
done. please take another look
8 years, 1 month ago (2012-11-20 06:04:27 UTC) #5
hashimoto
looks better, thanks https://codereview.chromium.org/11316097/diff/7002/chrome/browser/google_apis/gdata_wapi_url_util.cc File chrome/browser/google_apis/gdata_wapi_url_util.cc (right): https://codereview.chromium.org/11316097/diff/7002/chrome/browser/google_apis/gdata_wapi_url_util.cc#newcode108 chrome/browser/google_apis/gdata_wapi_url_util.cc:108: if (directory_resource_id.empty()) { How about putting ...
8 years, 1 month ago (2012-11-20 06:07:35 UTC) #6
satorux1
http://codereview.chromium.org/11316097/diff/7002/chrome/browser/google_apis/gdata_wapi_url_util.cc File chrome/browser/google_apis/gdata_wapi_url_util.cc (right): http://codereview.chromium.org/11316097/diff/7002/chrome/browser/google_apis/gdata_wapi_url_util.cc#newcode108 chrome/browser/google_apis/gdata_wapi_url_util.cc:108: if (directory_resource_id.empty()) { On 2012/11/20 06:07:35, hashimoto wrote: > ...
8 years, 1 month ago (2012-11-20 06:13:28 UTC) #7
hashimoto
lgtm http://codereview.chromium.org/11316097/diff/12002/chrome/browser/google_apis/gdata_wapi_url_util.cc File chrome/browser/google_apis/gdata_wapi_url_util.cc (right): http://codereview.chromium.org/11316097/diff/12002/chrome/browser/google_apis/gdata_wapi_url_util.cc#newcode107 chrome/browser/google_apis/gdata_wapi_url_util.cc:107: } else if (start_changestamp > 0) { Ah, ...
8 years, 1 month ago (2012-11-20 06:16:06 UTC) #8
satorux1
http://codereview.chromium.org/11316097/diff/12002/chrome/browser/google_apis/gdata_wapi_url_util.cc File chrome/browser/google_apis/gdata_wapi_url_util.cc (right): http://codereview.chromium.org/11316097/diff/12002/chrome/browser/google_apis/gdata_wapi_url_util.cc#newcode107 chrome/browser/google_apis/gdata_wapi_url_util.cc:107: } else if (start_changestamp > 0) { On 2012/11/20 ...
8 years, 1 month ago (2012-11-20 06:27:33 UTC) #9
hashimoto
8 years, 1 month ago (2012-11-20 06:28:31 UTC) #10
lgtm++, thanks

Powered by Google App Engine
This is Rietveld 408576698