[webkit-reviews] review denied: [Bug 231101] [WinCairo] Support run-jsc-stress-tests without posix commands : [Attachment 440092] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 5 14:36:43 PDT 2021


Adrian Perez <aperez at igalia.com> has denied Stephan Szabo
<stephan.szabo at sony.com>'s request for review:
Bug 231101: [WinCairo] Support run-jsc-stress-tests without posix commands
https://bugs.webkit.org/show_bug.cgi?id=231101

Attachment 440092: Patch

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




--- Comment #6 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 440092
  --> https://bugs.webkit.org/attachment.cgi?id=440092
Patch

Hello, and thanks for the patch. I think this is going in the right
direction, but I have a couple of suggestions to make it even better.
Please read on below :)

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

> Tools/Scripts/run-jsc-stress-tests:2334
> +    def each_line

I suppose this function could be just a function; there is no
need to put it inside a class, right?

> Tools/Scripts/run-jsc-stress-tests:2367
> +	       statusFileEnumerator.each_line {

I would completely remove the usage of the “find” command and use the
same code that uses Ruby functions to locate and read the result files.
The advantages would be having only one code path to maintain that works
for all ports, and avoiding spawning a bunch of subprocesses here.


More information about the webkit-reviews mailing list