On Wed, Aug 27, 2008 at 3:28 PM, Michael Nordman <[EMAIL PROTECTED]> wrote:
> ------------------------------------
> Line 319: // Hooray, it succeeded. Nothing more to do here.
> maybe invert the test, LOG("something went wrong") if anything doesn't workOn the flip side, it's nice that the current version uses parallel logic in Step 1 and Step 2. Feels like it could go either way. I'm leaning toward leaving as-is. > ------------------------------------ > Line 337: File::Delete(full_path.c_str()); // ignore return value > As files are really deleted, we may be able to actually delete the > corresponding > row instead of leaving them marked as deleted. Not sure its worth it... just > throwing this out there. Yeah, not sure it's worth it. I also don't know the other SQL well enough to be sure it's 100% safe. Again, leaning toward leaving as-is. > ======================================================================== > http://mondrian.corp.google.com/file/8116546///depot/googleclient/gears/opensource/gears/base/common/permissions_db.cc?a=1 > File > //depot/googleclient/gears/opensource/gears/base/common/permissions_db.cc > (snapshot 1) > ------------------------------------ > Line 186: database_name_table_.DeleteOriginDatabases(origin); > maybe rename this to DeleteDatabasesForOrigin for consistency with the > language > used below Done. > ------------------------------------ > Line 192: } > At this point, we may be able to delete data directory for this origin all > together. > > There may be shortcut icons stored in here, otherwise Gears would have no > other > data within this directory. > > Deleting the directory if its empty (or contains only empty directories) would > be safe. I think I'd have to write such a function, and test it well. It doesn't seem worthwhile, since this only affects empty directories.
