[webkit-reviews] review denied: [Bug 27605] Improve git workflow by populating commit messages with ChangeLog entries. : [Attachment 33334] revised patch to handle top-level changes properly

[bugzilla-daemon at webkit.org]

Adam Treat <treat at kde.org> has denied Eli Fidler <eli at staikos.net>'s request
for review:
Bug 27605: Improve git workflow by populating commit messages with ChangeLog
entries.
https://bugs.webkit.org/show_bug.cgi?id=27605

Attachment 33334: revised patch to handle top-level changes properly
https://bugs.webkit.org/attachment.cgi?id=33334&action=review

------- Additional Comments from Adam Treat <treat at kde.org>
> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 26eae74..654f679 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,17 @@
> +2009-07-23  Eli Fidler  <eli.fidler at torchmobile.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Improve git workflow by populating commit messages with ChangeLog
entries.
> +	   https://bugs.webkit.org/show_bug.cgi?id=27605
> +
> +		add --[no-]write option to optionally output new ChangeLog
entries to
> +		stdout instead of modifying ChangeLog files
> +
> +		fix Torch Mobile copyright

Please fix the indentation.

> -#   Add command line option to put the ChangeLog into a separate
> -#	 file or just spew it out stdout.
> +#   Add command line option to put the ChangeLog into a separate file.

Hmm, so this functionality supposedly existed at one point...

> +    if ($prefixDir eq "top level") {
> +	   $sortKey = "";
> +    } elsif ($prefixDir eq "Tools") {
> +	   $sortKey = "-, just after top level";
> +    } elsif ($prefixDir eq "WebBrowser") {
> +	   $sortKey = lc "WebKit, WebBrowser after";
> +    } elsif ($prefixDir eq "WebCore") {
> +	   $sortKey = lc "WebFoundation, WebCore after";

I understand that this logic was taken from commit-log-editor.	It'd be nice
for an explanation of where 'WebBrowser' and 'WebFoundation' come from, but I
suspect only the Apple folks would be able to provide that.  Still, it is good
to match commit-log-editor.

The patch looks good minus the minor nits above.  One semi-ugly thing is that
prepare-ChangeLog also writes to stderr by default a bunch of verbosity.  It is
not important for our use case, but perhaps a header or a line break should be
written to stderr right before we write to stdout to separate the two.

r- for the ChangeLog above.

Cheers,
Adam