[webkit-reviews] review denied: [Bug 81317] webkit-file utility needs a command-line interface : [Attachment 134788] patch, requires 64149, 61773 and 82595

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Roland Steiner
<rolandsteiner at chromium.org>'s request for review:
Bug 81317: webkit-file utility needs a command-line interface
https://bugs.webkit.org/show_bug.cgi?id=81317

Attachment 134788: patch, requires 64149, 61773 and 82595
https://bugs.webkit.org/attachment.cgi?id=134788&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134788&action=review


For now I just commented on test files.

> Tools/ChangeLog:11
> +	   * Scripts/webkit-file: Added.

Please explain what each file is for.

> Tools/ChangeLog:49
> +	   * Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:
Added.
> +	   * Scripts/webkitpy/common/project/unittest_files/Qt-add-file.pro:
Added.
> +	   * Scripts/webkitpy/common/project/unittest_files/Qt-sort.pro: Added.

> +	   * Scripts/webkitpy/common/project/unittest_files/Qt-unsorted.pri:
Added.
> +	   * Scripts/webkitpy/common/project/unittest_files/Qt-unsorted.pro:
Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/Xcode-add-file.pbxproj: Added.
> +	   * Scripts/webkitpy/common/project/unittest_files/Xcode-sort.pbxproj:
Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/Xcode-sorted.pbxproj: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/Xcode-unsorted.pbxproj: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/Xcode-unsorted.xcodeproj/project
.pbxproj: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/existing-in-folder.
cpp: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/existing-in-folder.
h: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/existing.cpp:
Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/existing.h: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/subfolder/existing-
in-dir.cpp: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-project/subfolder/existing-
in-dir.h: Added.
> +	   *
Scripts/webkitpy/common/project/unittest_files/test-target-Info.plist: Added.

- What does -sort.*, -unsorted.* and -add-file.* represent?

- Why doesn't IDL files appear in these test files other than
CMake-unsorted.txt?

> Tools/Scripts/webkitpy/common/project/options.py:189
> -		   self._verbosity = CommonOptions.VERBOSITY_VERY_VERBOSE
> +		   self._verbosity = CommonOptions.VERBOSITY_DEBUG

Nit: What is the change for?

> Tools/Scripts/webkit-file:41
> +    #webcore_setup = WebCoreDebugSetup()

Nit: Remove this line.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:31
> +    LIST(APPEND WebCore_SOURCES

The corresponding ')' is missing.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:39
> +    LIST(APPEND WebCore_SOURCES

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:44
> +LIST(APPEND WebCore_SOURCES

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:50
> +SET(WebCore_HEADERS

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:59
> +    LIST(APPEND WebCore_HEADERS

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:66
> +    LIST(APPEND WebCore_HEADERS

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:73
> +LIST(APPEND WebCore_HEADERS

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:82
> +    LIST(APPEND WebCore_HEADERS

Ditto.

> Tools/Scripts/webkitpy/common/project/unittest_files/CMake-unsorted.txt:85
> +    LIST(APPEND WebCore_SOURCES

Ditto.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-project/existing-in-f
older.h:36
> +
> +

Nit: Remove extra empty lines.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-project/existing.cpp:
32
> +

Ditto.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-project/existing.h:36

> +
> +

Ditto.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-project/subfolder/exi
sting-in-dir.cpp:32
> +

Ditto.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-project/subfolder/exi
sting-in-dir.h:36
> +
> +

Ditto.

>
Tools/Scripts/webkitpy/common/project/unittest_files/test-target-Info.plist:18
> +	<string>????</string>

(I am not sure what it is but) is '????' expected?