[webkit-reviews] review denied: [Bug 36568] Support BlobBuilder defined in FileAPI/FileWriter : [Attachment 52206] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 31 14:21:12 PDT 2010


Dmitry Titov <dimich at chromium.org> has denied Kinuko Yasuda
<kinuko at chromium.org>'s request for review:
Bug 36568: Support BlobBuilder defined in FileAPI/FileWriter
https://bugs.webkit.org/show_bug.cgi?id=36568

Attachment 52206: Patch
https://bugs.webkit.org/attachment.cgi?id=52206&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
r-, due to the size of the patch.

It is very hard to review 80Kb patch, and if the review happens, the quality of
it usually suffers. We ask contributors to come up with smaller patches,
especially if it is relatively easy to do.

In this case, it's possible to structure the work like this:
1. Have an uber-bug which says "Implement BlobBuilder" and serves as umbrella
for other bugs/patches.
2. Optionally attach the uber-patch with prototype into that bug.
3. Split work - in this case, it could be implementation of internal
implementation objects, like FileBlob, then JSC bindings with a test or two,
then v8 bindings, then more tests.
4. Have separate bugs with those smaller patches, referrign to the umbrella
bug. Normally, we have 1 patch per bug.

I myself didn't appreciate much the 'smaller patches' concept until I became a
reviewer. It takes about the same time to carefully review the patch as it
would take to write it. In the case of a big patch, the process repeats on
every iteration. It is really better and faster to come up with smaller
patches.


More information about the webkit-reviews mailing list