[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