[webkit-dev] Database in Worker context
dimich at chromium.org
Thu Jan 14 15:50:07 PST 2010
+1 to small patches.
I've reviewed a couple of iterations of the patch in
https://bugs.webkit.org/show_bug.cgi?id=22725 and it was so far manageable,
although it could be split in 2 at least (inheritance change and
WorkerThread termination change). For future patches, definitely lets split
into smaller ones, 60kb is on a biggish side.
There is no DatabaseManager class anymore since the database-supporting
methods are being moved to ScriptExecutionContext from which both Document
and WorkerContext are derived from. Having a 'Context' to own another
'Manager' seemed like too much layers.
I lean to r+ the first patch after latest set of notes is addressed, more
eyes are always welcome though :-)
On Thu, Jan 14, 2010 at 3:06 PM, Darin Adler <darin at apple.com> wrote:
> On Jan 14, 2010, at 2:35 PM, Eric Uhrhane wrote:
> > I think it would have been hard to break this chunk any smaller.
> Any time you have a chunk and you’re wondering how it could be broken up
> smaller, please feel free to ask that question after cc'ing me on a bug.
> Maciej and I, in particular, have a lot of experience with this, much of
> dating from the first year of the Safari project. I made changes that were
> too large and learned “no apologies” strategies to break them up into much
> smaller pieces that were easier to get right and to read and review.
> -- Darin
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev