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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=20919

Attachment 45242: [PATCH] Proposed fix
https://bugs.webkit.org/attachment.cgi?id=45242&action=review

------- 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);

Ditto.

review- so you can fix these minor quibbles