Bug 28477 - WebKit needs a changelogs.py to hold changelog-related code
Summary: WebKit needs a changelogs.py to hold changelog-related code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 26533
  Show dependency treegraph
 
Reported: 2009-08-19 17:39 PDT by Eric Seidel (no email)
Modified: 2009-08-20 22:44 PDT (History)
1 user (show)

See Also:


Attachments
Patch v1 (17.57 KB, patch)
2009-08-19 17:41 PDT, Eric Seidel (no email)
levin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-08-19 17:39:05 PDT
WebKit needs a changelogs.py to hold changelog-related code

This is just a first-pass.  We really need full-blown changelog parsing.  I have several partial implementations of changelog parsing in other scripts, including attached to bug 26533.

This module is just moving the bugzilla-tool changelog code to a module and making it testable!
Comment 1 Eric Seidel (no email) 2009-08-19 17:41:02 PDT
Created attachment 35167 [details]
Patch v1
Comment 2 Eric Seidel (no email) 2009-08-19 17:49:03 PDT
I think eventually the ChangeLog class will change to be more stateful.  Right
now it reads the ChangeLog from disk on every command.  Eventually we'll
probably have explicit read/write commands and keep the ChangeLog in memory in
a parsed form.  Not sure.
Comment 3 David Levin 2009-08-20 22:36:13 PDT
Comment on attachment 35167 [details]
Patch v1

A few things to consider..

> diff --git a/WebKitTools/Scripts/modules/changelogs.py
b/WebKitTools/Scripts/modules/changelogs.py
> +# Copyright (c) 2009, Google Inc. All rights reserved.
(C)

> +import fileinput # inplace file editing for set_reviewer_in_changelog
> +import re
> +

> +# This doesn't really belong in this file, but we don't have a better home
for it yet.
FIXME:

> +            if ChangeLog.date_line_regexp.match(line):
> +                return ''.join(entry_lines[0:-1]) # Remove the extra newline
at the end

Indexes at the boundary need not be listed, so it could be
 return ''.join(entry_lines[:-1]) # Remove the extra newline at the end.

However, I feel mildly paranoid about the blank line, so why not be slight more
robust line this?

 if not entry_lines[-1].strip():
     entry_lines = entry_lines[:-1]
 return ''.join(entry_lines) # Remove the extra newline at the end.



> +    def update_for_revert(self, revision):
> +        reviewed_by_regexp = re.compile('Reviewed by NOBODY \(OOPS!\)\.')
> +        removing_boilerplate = False
> +        # inplace=1 creates a backup file and re-directs stdout to the file
> +        for line in fileinput.FileInput(self.path, inplace=1):
> +            if re.search(reviewed_by_regexp, line):
reviewed_by_regexp.search(...

> +                print re.sub(reviewed_by_regexp, "No review, rolling out
r%s." % revision, line),
reviewed_by_regexp.sub(...



> +                print "        %s\n" % view_source_url(revision)
> +                # Remove all the ChangeLog boiler plate between the Reviewed
by line and the first changed file.
s/boiler plate/boilerplate/
Comment 4 David Levin 2009-08-20 22:37:38 PDT
Summarizing some lost state (due to bugzilla db corruption):

   Later Eric submitted another patch to address several of the issues which I then r+'ed (but I didn't find that it was checked in).
Comment 5 Eric Seidel (no email) 2009-08-20 22:44:33 PDT
http://trac.webkit.org/changeset/47603