[webkit-reviews] review granted: [Bug 183605] [WinCairo] Fix incompatibility of run-webkit-httpd on native Windows. : [Attachment 336083] PATCH

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 183605: [WinCairo] Fix incompatibility of run-webkit-httpd on native
Windows.
https://bugs.webkit.org/show_bug.cgi?id=183605

Attachment 336083: PATCH

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




--- Comment #22 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 336083
  --> https://bugs.webkit.org/attachment.cgi?id=336083
PATCH

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

> Tools/ChangeLog:8
> +	   To run Apapche HTTPD from the native Windows command line.

Apapche => Apache

This description and the bug title are insufficient. Please explain what the
incompatibility is and what is the fix. Even better, it should explain the
difference between native Windows and Cygwin that necessitated making this
change.

>> Tools/Scripts/webkitpy/common/system/filesystem.py:-194
>> -			os.rmdir(self._directory_path)
> 
> This fails when directory is not empty. That's not the purpose of this code.

That is the purpose of this code: to fail when the directory is not empty. This
purpose was questioned in the FIXME above it. Your proposed change signifies
that we are answering the question posed in the FIXME to the effect of "Yes, we
should delete non-empty directories." Therefore we should remove the FIXME
comment.

> Tools/Scripts/webkitpy/layout_tests/servers/run_webkit_httpd.py:114
> +    """Python equivalent of tail"""

tail => tail(1)

OR to be formal TAIL(1). Either notation helps convey that we are referring to
the tail command line tool whose man page is in section 1.