[webkit-reviews] review granted: [Bug 179633] [css-grid] Refactoring and new grid utility class : [Attachment 327373] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 20 16:22:48 PST 2017


Darin Adler <darin at apple.com> has granted Javier Fernandez
<jfernandez at igalia.com>'s request for review:
Bug 179633: [css-grid] Refactoring and new grid utility class
https://bugs.webkit.org/show_bug.cgi?id=179633

Attachment 327373: Patch

https://bugs.webkit.org/attachment.cgi?id=327373&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 327373
  --> https://bugs.webkit.org/attachment.cgi?id=327373
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327373&action=review

The name "Utils" goes against WebKit programming tradition; we try to steer
clear of the concept "utilities" and we also don’t abbreviate words with things
like "utils" if we can avoid it. It’s also not best style to use a class just
as a namespace for some functions. I suggest just naming this GridLayout, or if
that really seems wrong, GridLayoutFunctions (as with LengthFunctions). And we
should use a namespace rather than a class.

Otherwise this seems fine, but please upload a version that builds.

> Source/WebCore/rendering/GridLayoutUtils.cpp:25
> + */
> +#include "config.h"

Missing blank line.


More information about the webkit-reviews mailing list