[webkit-reviews] review denied: [Bug 29749] Integrate chromium into update-webkit and build-webkit : [Attachment 40273] Patch 4

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Yaar Schnitman
<yaar at chromium.org>'s request for review:
Bug 29749: Integrate chromium into update-webkit and build-webkit
https://bugs.webkit.org/show_bug.cgi?id=29749

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

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
(In reply to comment #13)
> All feedback incorporated into patch 4, except:
> 
> > > +} elsif ($chromium) {
> > > +    system("perl", "WebKitTools/Scripts/update-webkit-chromium") == 0 or
die;
> 
> isChromium doesn't work in update-webkit. If this is critical, please advise
me
> of how to make isChromium work.

Like I said in Comment #10, "Just add the call to determineIsChromium() before
the GetOptions() call...":

--- a/WebKitTools/Scripts/update-webkit
+++ b/WebKitTools/Scripts/update-webkit
@@ -44,6 +44,8 @@ sub normalizePath($);
 my $quiet = '';
 my $showHelp;
 
+determineIsChromium();
+
 my $getOptionsResult = GetOptions(
     'h|help'  => \$showHelp,
     'q|quiet' => \$quiet,

That method parses @ARGV for "--chromium" and then removes it from @ARGV.  When
you later call isChromium(), the value is set to true if the --chromium switch
was parsed, else it returns false.

Please make this change and post the patch again.  Thanks for your patience!