[webkit-reviews] review denied: [Bug 80897] move calc*Value functions out from Length (and platform) : [Attachment 131803] ProposedPatch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Joe Thomas
<joethomas at motorola.com>'s request for review:
Bug 80897: move calc*Value functions out from Length (and platform)
https://bugs.webkit.org/show_bug.cgi?id=80897

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=131803&action=review


Looks generally good, a few comments.

> Source/WebCore/css/LengthFunctions.h:3
> +/*
> + * Copyright (c) 2012 Motorola Mobility, Inc. All rights reserved.
> + *

Since this is copy code you should maintain the original license and copyrights
from Length.h and just add a new (c) line.

> Source/WebCore/css/LengthFunctions.h:36
> +int calculateValue(Length, int maxValue, bool roundPercentages = false);
> +int calculateMinValue(Length, int maxValue, bool roundPercentages = false);
> +float calculateFloatValue(Length, float maxValue);
> +float calculateFloatValue(Length, int maxValue);

It would be better to put everything to header as the current implementations
are inlined in Length.h. Making them regular functions might not affect
performance but it would be safer to do that change separately.

Less generic names would be nice though I don't have good suggestions.

calculateMinValue, minValue, maxValue should be renamed with "minimum" and
"maximum"