[webkit-reviews] review denied: [Bug 74253] [Refactoring] Move top-level code to generate a new ChangeLog into a method : [Attachment 118705] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 74253: [Refactoring] Move top-level code to generate a new ChangeLog into a
method
https://bugs.webkit.org/show_bug.cgi?id=74253

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

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=118705&action=review


r- for style issues with variable names, but otherwise this looks good.

> Tools/Scripts/prepare-ChangeLog:171
> +my $added_regression_tests = generateFileList(@changed_files,
@conflict_files, %function_lists);

The $aded_regression_tests variable name does not match the prevalent style for
variable names in other Perl scripts (and Python and C++ code).  I would expect
it to be $addedRegressionTests.

Why is this style being used for these changes?

> Tools/Scripts/prepare-ChangeLog:453
> +    my ($prefixes, $files_in_change_log, $added_regression_tests,
$function_lists) = @_;

CamelCase should be used for variables instead of underscores.

> Tools/Scripts/prepare-ChangeLog:1561
> +    my @added_regression_tests;

Should be @addedRegressionTests.

> Tools/Scripts/prepare-ChangeLog:1614
> +		   push @added_regression_tests, $file

Ditto.

> Tools/Scripts/prepare-ChangeLog:1631
> +    return \@added_regression_tests;

Ditto.