Bug 46058 - Add infrastructure towards detecting change log diffs that aren't at the top of the ChangeLog
Summary: Add infrastructure towards detecting change log diffs that aren't at the top ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks: 46061
  Show dependency tree / graph
 
Reported: 2010-09-19 12:50 PDT by Daniel Bates
Modified: 2010-10-05 21:59 PDT (History)
4 users (show)

See Also:

Attachments
Patch (15.51 KB, patch)
2010-09-19 14:21 PDT, Daniel Bates 		ddkilzer: review+
ddkilzer: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>