[webkit-reviews] review denied: [Bug 118367] Nix upstreaming - Adding build files and supporting scripts : [Attachment 206019] Patch

[bugzilla-daemon at webkit.org]

Gyuyoung Kim <gyuyoung.kim at samsung.com> has denied Marcelo Lira
<marcelo.lira at openbossa.org>'s request for review:
Bug 118367: Nix upstreaming - Adding build files and supporting scripts
https://bugs.webkit.org/show_bug.cgi?id=118367

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

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=206019&action=review


It looks you need to take care of placing files by alphabetic order first.

> Source/WebCore/PlatformNix.cmake:2
> +    "${WEBCORE_DIR}/platform/nix"

Please place files according to alphabetic order.

> Source/WebCore/PlatformNix.cmake:20
> +    page/nix/EventHandlerNix.cpp

It would be good if you do grouping as below,

page/nix/EventHandlerNix.cpp
page/TouchAdjustment.cpp

platform/Cursor.cpp
platform/ContextMenuNone.cpp

> Source/cmake/OptionsCommon.cmake:6
> +

Unneeded line.

> Tools/Scripts/build-webkit:153
> +  --nix			       Build the Nix port

Wrong alphabetic order.

> Tools/Scripts/run-javascriptcore-tests:60
> +    # Tests failing when running in BRT timezone

I'm not sure if you can skip these tests without considering other ports.

> Tools/Scripts/webkitdirs.pm:993
> +    # The presence of QTDIR only means Qt if --gtk or --efl or --blackberry
or --wincairo or --nix are not on the command-line

Wrong line ?

> Tools/Scripts/webkitdirs.pm:1154
> +}

Missing a new line.