[Webkit-unassigned] [Bug 15334] Split ResourceResponse into platform specific files

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=15334


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16487|review?                     |review-
               Flag|                            |




------- Comment #5 from darin at apple.com  2007-10-04 09:13 PDT -------
(From update of attachment 16487)
This patch would be easier to review if you did an "svn move" of
ResourceResponse.cpp to ResourceResponseBase.cpp -- the patch would then show
only diffs, rather than showing the entire file as changed and we also wouldn't
lose history on the files. svn-create-patch does a good job of handling cases
like that.

+    void doUpdateResourceResponse() {
+        notImplemented();
+    }

We put braces on the second line, not the first.

+   
const_cast<ResourceResponse&>(asResourceResponse()).doUpdateResourceResponse();

Why use const_cast here? Could we instead make the relevant members all be
mutable? I'm always uncomfortable with the "do" prefix in cases like this.
Perhaps we could come up with a more descriptive name for the function that
does the actual work.

+    //Used when response is initialized from a platform representation

We normally put a space after the "//".

We normally don't put a blank line after "protected:" etc.

I'm going to mark this review- because I'd like to instead see a version of the
patch that does "svn move".


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.