[webkit-reviews] review denied: [Bug 53591] print-vse-failure-logs: "Found 0 VSE Build Logs" and "WebKit/WebKitBuild/obj" is not there : [Attachment 83414] *This* is the patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Felipe <f.dachshund at gmail.com>'s
request for review:
Bug 53591: print-vse-failure-logs: "Found 0 VSE Build Logs" and
"WebKit/WebKitBuild/obj" is not there
https://bugs.webkit.org/show_bug.cgi?id=53591

Attachment 83414: *This* is the patch
https://bugs.webkit.org/attachment.cgi?id=83414&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=83414&action=review

How do we test this?  We have unit testing for Python and perl, seems we should
use it.

> Tools/Scripts/build-webkit:590
> +	       if ($configuration eq 'Debug') {
> +		   system(File::Spec->catfile($scriptDir,
"print-vse-failure-logs"), '--debug');
> +	       } else {
> +		   system(File::Spec->catfile($scriptDir,
"print-vse-failure-logs"), '--release');
> +	       }

I'm pretty sure that wkdirs.pm has a function for returning --debug or
--release from the $configuration information.

> Tools/Scripts/print-vse-failure-logs:126
> +    elif len(sys.argv) == 2:
> +	   if sys.argv[1] != "--release" and sys.argv[1] != "--debug":
> +	       usage()
> +	   option = sys.argv[1]

Seems we should consider using optparse.  It's very easy to do.