Bug 76956

Summary: Support a suffix on ChangeLog filenames based on a configuration file
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: Tools / TestsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, haraken, webkit.review.bot
Priority: P2 Keywords: EasyFix, InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Benjamin Poulain 2012-01-24 16:19:05 PST
<rdar://problem/10585009>

For some branches, we have internal WebKit repositories with ChangeLog, but we do not want to mess up with the main ChangeLog. To track the change, we have a ChangeLog with a suffix (e.g.: ChangeLog-533.21.1).

The current solution is to hack the tools and add some suffix there.

I suggest we do that in a cleaner way and have some "config file" to add an arbitrary suffix to the ChangeLog. That way we can just drop the "config file" in the repository and everything would work directly.
Comment 1 Benjamin Poulain 2012-01-24 16:47:19 PST
Created attachment 123842 [details]
Patch
Comment 2 Benjamin Poulain 2012-01-24 16:48:26 PST
Warning: I am not at all familiar with Perl. Please be careful when reviewing :)
Comment 3 Kentaro Hara 2012-01-25 17:53:10 PST
Comment on attachment 123842 [details]
Patch

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

The approach looks OK to me, but I am not sure if the scripts we need to modify are only resolve-ChangeLogs, prepare-ChangeLog and commit-log-editor? There are other scripts that contain "ChangeLog". ($ grep -rl '"ChangeLog"' Tools/Scripts/*)

> Tools/Scripts/VCSUtils.pm:1765
> +    open FILE, File::Spec->catfile($rootPath, "ChangeLogSuffix") or die "Could not open $changeLogSuffixFile: $!";

This should be

    open FILE, $changeLogSuffixFile or die "Could not open $changeLogSuffixFile: $!";

> Tools/Scripts/VCSUtils.pm:1767
> +    my $changeLogSuffix = <FILE>;
> +    close FILE;

Let's strip trailing \n, like this:

    my $changeLogSuffix = <FILE>;
    chomp $changeLogSuffix;
    close FILE;
Comment 4 Benjamin Poulain 2012-01-26 14:12:03 PST
Created attachment 124178 [details]
Patch
Comment 5 Benjamin Poulain 2012-01-26 14:15:52 PST
Thanks for looking into this. I fixed the error you discovered.

> View in context: https://bugs.webkit.org/attachment.cgi?id=123842&action=review
> 
> The approach looks OK to me, but I am not sure if the scripts we need to modify are only resolve-ChangeLogs, prepare-ChangeLog and commit-log-editor? There are other scripts that contain "ChangeLog". ($ grep -rl '"ChangeLog"' Tools/Scripts/*)

I intentionally ignore webkitpy and changed the minimal set of tools we need. This is solving one particular problem instead of fully supporting a complete new feature for the tools.
Comment 6 Kentaro Hara 2012-01-26 14:17:26 PST
(In reply to comment #5)
> > The approach looks OK to me, but I am not sure if the scripts we need to modify are only resolve-ChangeLogs, prepare-ChangeLog and commit-log-editor? There are other scripts that contain "ChangeLog". ($ grep -rl '"ChangeLog"' Tools/Scripts/*)
> 
> I intentionally ignore webkitpy and changed the minimal set of tools we need. This is solving one particular problem instead of fully supporting a complete new feature for the tools.

Makes sense. Thank you for the patch!
Comment 7 WebKit Review Bot 2012-01-26 15:29:30 PST
Comment on attachment 124178 [details]
Patch

Clearing flags on attachment: 124178

Committed r106054: <http://trac.webkit.org/changeset/106054>
Comment 8 WebKit Review Bot 2012-01-26 15:29:34 PST
All reviewed patches have been landed.  Closing bug.