[webkit-reviews] review denied: [Bug 35213] [chromium] hardcoded gcc path breaks solaris build : [Attachment 49255] fix changelog formatting, again

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied electricmonopole at gmail.com's
request for review:
Bug 35213: [chromium] hardcoded gcc path breaks solaris build
https://bugs.webkit.org/show_bug.cgi?id=35213

Attachment 49255: fix changelog formatting, again
https://bugs.webkit.org/attachment.cgi?id=49255&action=review

------- Additional Comments from David Levin <levin at chromium.org>
> Index: ChangeLog
> +2010-02-22  James Choi  <jchoi42 at pha.jhu.edu>
> +
> +	   Change hardcoded gcc paths to be Solaris friendly
> +	   https://bugs.webkit.org/show_bug.cgi?id=35214
> +
> +	   * WebCore/bindings/scripts/CodeGeneratorObjC.pm,
WebCore/bindings/scripts/IDLParser.pm, WebCore/css/make-css-file-arrays.pl,
WebCore/dom/make_names.pl

Each file typically has its own line with a * before it. I imaging that is how
prepare-ChangeLog generated it but this looks odd to me.


> Index: WebCore/bindings/scripts/CodeGeneratorObjC.pm

In each of these changes, the line containing gcc has been duplicated
completely. It would be better if the change was only about the gcc location.
For example,

    my $gccLocation = "";
    if (($Config::Config{'osname'}) =~ /solaris/i) {
	$gccLocation = "/usr/sfw/bin/gcc";
    } else {
	$gccLocation = "/usr/sfw/bin/gcc";
    }
    open FILE, "-|", $gccLocation, "-E", "-P", "-x", "objective-c", 
	(map { "-D$_" } split(/ +/, $defines)), "-DOBJC_CODE_GENERATION",
$fileName or die "Could not open $fileName";


Also please note the spaces around the =~ and before the {.