[webkit-reviews] review denied: [Bug 20919] Constrain the number of messages the inspector shows : [Attachment 45242] [PATCH] Proposed fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Dec 19 11:29:47 PST 2009

Darin Adler <darin at apple.com> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 20919: Constrain the number of messages the inspector shows

Attachment 45242: [PATCH] Proposed fix

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   m_expiredConsoleMessageCount += 100;
> +	   for (size_t i = 0; i < 100; ++i)
> +	       delete m_consoleMessages[i];
> +	   m_consoleMessages.remove(0, 100);

Should use a named constant for 100 instead of repeating it 3 times. In fact, I
think it would be good to put that constant up at the top of the file along
with maximumConsoleMessages.

> +void InspectorFrontend::updateConsoleMessageExpiredCount(const int count)

The const here should be left out. All it does is prevent the function from
modifying its local copy of the argument internally. This is not something we
should do in WebKit code.

> +	   void updateConsoleMessageExpiredCount(const int count);


review- so you can fix these minor quibbles

More information about the webkit-reviews mailing list