Issue 11418084: google_apis: Introduce GDataWapiUrlGenerator class

Issue 11418084: google_apis: Introduce GDataWapiUrlGenerator class (Closed)

Created:
8 years, 1 month ago by satorux1

Modified:
8 years, 1 month ago

Reviewers:
hashimoto

CC:
chromium-reviews, achuith+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

google_apis: Introduce GDataWapiUrlGenerator class The class allows us to inject a base URL for testing. Some TODOs were left intentionally to make the patch in a manageable size. BUG=161903 TEST=Drive works as before Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=168963

Patch Set 1 : polish #

Total comments: 4

Patch Set 2 : rebase #

Patch Set 3 : address comments #

Total comments: 2

Patch Set 4 : address comments #

Created: 8 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+223 lines, -132 lines)			Patch
M	chrome/browser/google_apis/gdata_wapi_operations.h	View	1 2	13 chunks	+14 lines, -1 line	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_operations.cc	View		9 chunks	+23 lines, -11 lines	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_service.h	View	1 2	2 chunks	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_service.cc	View	1 2	8 chunks	+18 lines, -6 lines	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_url_util.h	View	1 2	2 chunks	+58 lines, -38 lines	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_url_util.cc	View	1 2 3	4 chunks	+48 lines, -31 lines	0 comments	Download
M	chrome/browser/google_apis/gdata_wapi_url_util_unittest.cc	View	1 2	6 chunks	+58 lines, -45 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

hashimoto

Please fix the compile error http://codereview.chromium.org/11418084/diff/2001/chrome/browser/google_apis/gdata_wapi_service.cc File chrome/browser/google_apis/gdata_wapi_service.cc (right): http://codereview.chromium.org/11418084/diff/2001/chrome/browser/google_apis/gdata_wapi_service.cc#newcode239 chrome/browser/google_apis/gdata_wapi_service.cc:239: new CreateDirectoryOperation( nit: Can't ...

8 years, 1 month ago (2012-11-20 09:10:17 UTC) #2

satorux1

Made some other changes too: 1) GDataUrlGenerator now takes a GURL rather than a std::string ...

8 years, 1 month ago (2012-11-21 00:34:18 UTC) #3

hashimoto

lgtm https://codereview.chromium.org/11418084/diff/14001/chrome/browser/google_apis/gdata_wapi_url_util.cc File chrome/browser/google_apis/gdata_wapi_url_util.cc (right): https://codereview.chromium.org/11418084/diff/14001/chrome/browser/google_apis/gdata_wapi_url_util.cc#newcode51 chrome/browser/google_apis/gdata_wapi_url_util.cc:51: const char kGetChangesListURL[] = nit: No need to ...

8 years, 1 month ago (2012-11-21 03:27:50 UTC) #4

satorux1

8 years, 1 month ago (2012-11-21 03:58:56 UTC) #5

https://codereview.chromium.org/11418084/diff/14001/chrome/browser/google_api...
File chrome/browser/google_apis/gdata_wapi_url_util.cc (right):

https://codereview.chromium.org/11418084/diff/14001/chrome/browser/google_api...
chrome/browser/google_apis/gdata_wapi_url_util.cc:51: const char
kGetChangesListURL[] =
On 2012/11/21 03:27:50, hashimoto wrote:
> nit: No need to have newline?

Done.

Expand Messages | Collapse Messages