Bug 76956 - Support a suffix on ChangeLog filenames based on a configuration file
Summary: Support a suffix on ChangeLog filenames based on a configuration file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords: EasyFix, InRadar
Depends on:
Blocks:
 
Reported: 2012-01-24 16:19 PST by Benjamin Poulain
Modified: 2012-01-26 15:29 PST (History)
3 users (show)

See Also:


Attachments
Patch (5.67 KB, patch)
2012-01-24 16:47 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (5.60 KB, patch)
2012-01-26 14:12 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

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