Issue 10204013: Move FindEntryDelegate and friends to separate file.

Issue 10204013: Move FindEntryDelegate and friends to separate file. (Closed)

Created:
8 years, 8 months ago by achuithb

Modified:
8 years, 8 months ago

Reviewers:
satorux1, zel

CC:
chromium-reviews, achuith+watch_chromium.org, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

More Reviews

Description

Move FindEntryDelegate and friends to seperate file. Move GDataFileSystem::UnsafeFindEntryByPath to GDataRootDirectory::FindEntryByPath. BUG=chromium-os:29232 TEST=compiles, tests pass. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=133889

Patch Set 1 #

Total comments: 1

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Total comments: 4

Patch Set 5 : change tense of comments #

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+191 lines, -160 lines)			Patch
A	chrome/browser/chromeos/gdata/find_entry_delegate.h	View		1 chunk	+76 lines, -0 lines	0 comments	Download
A	chrome/browser/chromeos/gdata/find_entry_delegate.cc	View	1	1 chunk	+41 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_file_system.h	View	1 2	4 chunks	+1 line, -43 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_file_system.cc	View	1 2	6 chunks	+4 lines, -107 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_files.h	View	1 2 3 4	6 chunks	+15 lines, -10 lines	0 comments	Download
M	chrome/browser/chromeos/gdata/gdata_files.cc	View	1 2	2 chunks	+52 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

satorux1

http://codereview.chromium.org/10204013/diff/1/chrome/browser/chromeos/gdata/find_entry_delegate.cc File chrome/browser/chromeos/gdata/find_entry_delegate.cc (right): http://codereview.chromium.org/10204013/diff/1/chrome/browser/chromeos/gdata/find_entry_delegate.cc#newcode39 chrome/browser/chromeos/gdata/find_entry_delegate.cc:39: } nit: remove {} per chromium style guide.

8 years, 8 months ago (2012-04-24 17:48:01 UTC) #3

achuithb

Ignore this for now. Going to focus on landing the other patch first.

8 years, 8 months ago (2012-04-24 20:16:30 UTC) #4

satorux1

LGTM. less code in gdata_file_system.cc is nice! I'd be happier if you could write some ...

8 years, 8 months ago (2012-04-25 06:01:36 UTC) #6

achuithb

8 years, 8 months ago (2012-04-25 10:05:35 UTC) #7

Thanks for the review!

http://codereview.chromium.org/10204013/diff/20001/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_files.cc (right):

http://codereview.chromium.org/10204013/diff/20001/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_files.cc:499: }
On 2012/04/25 06:01:37, satorux1 wrote:
> would it make sense to write a unit test for this function? Now it's easier to
> write a focused unit test.

I'm hoping to get rid of this entire function when I replace it with the
database lookup.

http://codereview.chromium.org/10204013/diff/20001/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_files.h (right):

http://codereview.chromium.org/10204013/diff/20001/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_files.h:426: // Search for |file_path|
triggering callback in |delegate|.
On 2012/04/25 06:01:37, satorux1 wrote:
> Search -> Searches
> 
> While you are at it, could you fix other function comments in this file?

Done.

Expand Messages | Collapse Messages