[webkit-reviews] review granted: [Bug 174197] Make prepare-ChangeLog -g <commit> generate a more standard ChangeLog entry. : [Attachment 314709] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Emilio Cobos Álvarez
<ecobos at igalia.com>'s request for review:
Bug 174197: Make prepare-ChangeLog -g <commit> generate a more standard
ChangeLog entry.
https://bugs.webkit.org/show_bug.cgi?id=174197

Attachment 314709: Patch

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




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 314709
  --> https://bugs.webkit.org/attachment.cgi?id=314709
Patch

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

Seems fine.

Since non-perl programmers can’t read the code to strip off the first line
anyway, I would consider doing it with a single regular expression like this:

    ($bugDescription, $description)
	= ($description =~ /^(?:\s*(.*)\n)?(?:\s*\n)*(.*)/)
	if !$bugDescription && $description;

Could write that all on one line, too, if you prefer. Note that my version
strips multiple blank lines, not just one. Could use a "?" instead of a "*" to
strip only one.

> Tools/Scripts/prepare-ChangeLog:677
> +	       my @lines = split(/\n/, $description);

I think it’s more efficient to split on "\n" rather than /\n/ but not sure. Of
course, if you use my alternative version you won’t need that.

> Tools/Scripts/prepare-ChangeLog:679
> +	       $bugDescription =~ s/^\s*//g;

No need for the "g" here; this only applies once, not multiple times. Also, I
would have used + rather than * myself.