Bug 46058

Summary: Add infrastructure towards detecting change log diffs that aren't at the top of the ChangeLog
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Tools / TestsAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, ddkilzer, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 46061    
Attachments:
Description Flags
Patch ddkilzer: review+, ddkilzer: commit-queue-

Description Daniel Bates 2010-09-19 12:50:19 PDT
Towards checking that a change log diff inserts an entry at the top of the ChangeLog file (and possibly have the caller print a warning if it does not), we need to modify VCSUtils::fixChangeLogPatch() so that it returns a reference to a hash structure instead of a string that represents a diff. This will allow us to return additional information to the caller, such as whether the the change log diff inserts an entry at the top of the ChangeLog file.
Comment 1 Daniel Bates 2010-09-19 14:21:50 PDT
Created attachment 68036 [details]
Patch
Comment 2 Adam Barth 2010-09-26 22:28:26 PDT
This looks very useful.  Sadly, I don't speak Perl.
Comment 3 David Kilzer (:ddkilzer) 2010-10-03 20:23:17 PDT
Comment on attachment 68036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68036&action=review

> WebKitTools/Scripts/VCSUtils.pm:1575
> +        my $changeLog = fixChangeLogPatch($patch);

Variable name should note it's a reference to a hash.  Maybe $changeLogHash.

> WebKitTools/Scripts/svn-apply:319
> +            my $changeLog = fixChangeLogPatch($patch);

Ditto.

> WebKitTools/Scripts/svn-create-patch:236
> +        my $changeLog = fixChangeLogPatch($patch);

Ditto.

> WebKitTools/Scripts/svn-unapply:161
> +            my $changeLog = fixChangeLogPatch($patch);

Ditto.

> WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl:4
> +# Copyright (C) Research in Motion Limited 2010. All Rights Reserved.

Shouldn't the year of the copyright be after the "(C)"?

r=me with the variable name changes.
Comment 4 Daniel Bates 2010-10-05 19:54:16 PDT
(In reply to comment #3)
> (From update of attachment 68036 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68036&action=review
> 
> > WebKitTools/Scripts/VCSUtils.pm:1575
> > +        my $changeLog = fixChangeLogPatch($patch);
> 
> Variable name should note it's a reference to a hash.  Maybe $changeLogHash.
> 

Will change before I land.

> > WebKitTools/Scripts/svn-apply:319
> > +            my $changeLog = fixChangeLogPatch($patch);
> 
> Ditto.
> 

Will change before I land.

> > WebKitTools/Scripts/svn-create-patch:236
> > +        my $changeLog = fixChangeLogPatch($patch);
> 
> Ditto.
> 

Will change before I land.

> > WebKitTools/Scripts/svn-unapply:161
> > +            my $changeLog = fixChangeLogPatch($patch);
> 
> Ditto.
> 

Will change before I land.

> > WebKitTools/Scripts/webkitperl/VCSUtils_unittest/fixChangeLogPatch.pl:4
> > +# Copyright (C) Research in Motion Limited 2010. All Rights Reserved.
> 
> Shouldn't the year of the copyright be after the "(C)"?

The RIM copyright string uses this explicit order.
Comment 5 Daniel Bates 2010-10-05 21:59:49 PDT
Committed r69177: <http://trac.webkit.org/changeset/69177>