[webkit-reviews] review granted: [Bug 28168] commit-log-editor does not support all the email config that prepare-Changelog supports : [Attachment 34527] commit-log-editor does not support all the email config that prepare-Changelog supports

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 10 16:58:41 PDT 2009


Mark Rowe (bdash) <mrowe at apple.com> has granted Pierre d'Herbemont
<pdherbemont at free.fr>'s request for review:
Bug 28168: commit-log-editor does not support all the email config that
prepare-Changelog supports
https://bugs.webkit.org/show_bug.cgi?id=28168

Attachment 34527: commit-log-editor does not support all the email config that
prepare-Changelog supports
https://bugs.webkit.org/attachment.cgi?id=34527&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
> +sub gitConfig($)
> +{
> +    return unless $isGit;
> +    
> +    my ($config) = @_;
> +    
> +    my $result = `git config $config`;
> +    if (($? >> 8) != 0) {
> +	   $result = `git repo-config $config`;
> +    }
> +    chomp $result;
> +    return $result;
> +}

In theory "$? >> 8" would be more portable as exitStatus($?).

>  1;
> diff --git a/WebKitTools/Scripts/commit-log-editor
b/WebKitTools/Scripts/commit-log-editor
> index b783ccb..fe138ae 100755
> --- a/WebKitTools/Scripts/commit-log-editor
> +++ b/WebKitTools/Scripts/commit-log-editor
> @@ -171,7 +171,10 @@ for my $changeLog (@changeLogs) {
>  
>	       # Attempt to insert the "patch by" line, after the first blank
line.
>	       if ($previousLineWasBlank && $hasAuthorInfoToWrite && $lineCount
> 0) {
> -		   my $authorAndCommitterAreSamePerson = $email eq
$ENV{'EMAIL_ADDRESS'};
> +		   my $committerEmail = $ENV{CHANGE_LOG_EMAIL_ADDRESS}
> +				       || $ENV{EMAIL_ADDRESS}
> +				       || gitConfig("user.email");
> +		   my $authorAndCommitterAreSamePerson = $email eq
$committerEmail;

What will happen if the email address is not set via any of these means?  Will
it spew a perl warning and continue, or will it display a useful message like
prepare-ChangeLog does?  Should we consider removing the duplication of the
retrieving of these values by pushing them down in to a function?

r=me


More information about the webkit-reviews mailing list