The objective is to make prepare-ChangeLog a loadable Perl module for unit testing, which requires to remove top-level code and global variables. This bug is one of the incremental refactorings for that.
Created attachment 118705 [details] Patch
Comment on attachment 118705 [details] Patch 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.
Created attachment 118707 [details] Patch
(In reply to comment #2) > (From update of attachment 118705 [details]) > 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? Fixed it. Actually, I have been so far confused about the convention of variable names in Python and Perl scripts. (i.e. whether the WebKit coding style guide is applied to Python and Perl or not) because we can find many existing Python and Perl scripts that are using under_score_names. In case of ./prepare-ChangeLog, under_score_names appear to be being used a bit more than CamelCaseNames.
Comment on attachment 118707 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118707&action=review r=me with the subroutine name change. > Tools/Scripts/prepare-ChangeLog:451 > +sub generateNewChangeLog($$$\%) This should probably be named generateNewChangeLogs() since it creates more than one.
Created attachment 118708 [details] patch for commit
Comment on attachment 118708 [details] patch for commit Clearing flags on attachment: 118708 Committed r102537: <http://trac.webkit.org/changeset/102537>
Manually marking as RESOLVED/FIXED.