[Webkit-unassigned] [Bug 28675] Make prepare-ChangeLog notice property changes
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Aug 25 21:46:53 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=28675
David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #38585|review? |review-
Flag| |
--- Comment #4 from David Kilzer (ddkilzer) <ddkilzer at webkit.org> 2009-08-25 21:46:53 PDT ---
(From update of attachment 38585)
Thanks for taking this on! This has been a long time in coming.
> + open INFO, "$SVN diff '$file' |" or die;
Please use "DIFF" for the file handle name instead of "INFO" here.
> - next unless $status;
> + next unless $status && !(isUnmodifiedStatus($status) && isUnmodifiedStatus($propertyStatus));
This logic is too negative. Can we make it more positive? Also, I think you
need to check $propertyStatus since there are two cases (one for svn and one
for git) where it may not be set.
next if (!$status || isUnmodifiedStatus($status)) && (!$propertyStatus ||
isUnmodifiedStatus($propertyStatus));
This may also be fixed by setting $propertyStatus to a default of " ". That
may work better for the code following this statement.
> +sub propertyChangeDescription($)
> +{
> + my ($propertyChanges) = @_;
> +
> + my @properties = keys(%$propertyChanges);
> + my $property;
> + my $formatString;
> +
> + if (@properties == 1) {
> + my %operation = (
> + "A" => " Added %s property.",
> + "M" => " Modified %s property.",
> + "D" => " Removed %s property.",
> + );
> +
> + ($property) = @properties;
> + my $propertyChange = $propertyChanges->{$property};
> + $formatString = $operation{$propertyChange};
> + } else {
> + $formatString = " Changed properties.";
> + }
> +
> + return sprintf($formatString, $property);
> +}
I would rather see all property changes listed if there are more than one.
Please update this method to list all the property changes and just append the
formatted strings to a $result string.
r- to make the above changes.
Thanks!
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list