[webkit-reviews] review denied: [Bug 24593] Minor renaming in Chromium port's ResourceRequest.h : [Attachment 28612] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 13 16:45:51 PDT 2009


Darin Fisher (:fishd, Google) <darin at chromium.org> has denied John Abd-El-Malek
<jam at google.com>'s request for review:
Bug 24593: Minor renaming in Chromium port's ResourceRequest.h
https://bugs.webkit.org/show_bug.cgi?id=24593

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<darin at chromium.org>
>Index: WebCore/ChangeLog
...
>+2009-03-13  jabdelmalek  <set EMAIL_ADDRESS environment variable>

nit: need to add your email address.  also good to put your full name.

you can set the env vars REAL_NAME and EMAIL_ADDRESS to make this
automatic next time you run prepare-ChangeLog.

>+	  Reviewed by NOBODY (OOPS!).
>+
>+		https://bugs.webkit.org/show_bug.cgi?id=24593
>+
>+		Added requestorID so we can track the request to its WebView
without using frame (which was removed).
>+

nit: tabs need to be replaced with spaces


>Index: WebCore/platform/network/chromium/ResourceRequest.h
...
>+	      , m_requestorID(0)
>+	      , m_requestorPid(0)

webkit style would make this m_requestorPID, but even better (I think)
would be to go with m_requestorProcessID.  that way it is not as visually
confusing with m_requestorID.  verbose names is consistent with webkit
style ;-)


More information about the webkit-reviews mailing list