Bug 108788 - Don't update author info in PrepareChangeLog and allow users to skip the PrepareChangeLog step entirely.
Summary: Don't update author info in PrepareChangeLog and allow users to skip the Prep...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-03 21:28 PST by Timothy Loh
Modified: 2013-02-04 16:47 PST (History)
7 users (show)

See Also:


Attachments
Patch (9.60 KB, patch)
2013-02-03 21:43 PST, Timothy Loh
no flags Details | Formatted Diff | Diff
Patch (9.82 KB, patch)
2013-02-03 22:17 PST, Timothy Loh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Loh 2013-02-03 21:28:07 PST
Don't update author info in PrepareChangeLog and allow users to skip the PrepareChangeLog step entirely.
Comment 1 Timothy Loh 2013-02-03 21:43:37 PST
Created attachment 186300 [details]
Patch
Comment 2 Ryosuke Niwa 2013-02-03 21:50:13 PST
Comment on attachment 186300 [details]
Patch

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

> Tools/Scripts/webkitpy/tool/steps/preparechangelog.py:87
> -        new_date_line = new_entry.date_line()
> -        old_date_line = old_entry.date_line()
> -        if new_date_line != old_date_line:
> -            final_entry = final_entry.replace(old_date_line, new_date_line)
> +        final_entry = final_entry.replace(old_entry.date(), new_entry.date(), 1)

Why are we making this change? This patch needs per-function change log.

> Tools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py:99
> +                      ((4, 0, 0, 0), (0, 0, 0, 0), (0, 0, 0, 0)),
> +                      ((5, 0, 0, 0), (0, 0, 0, 0), (1, 0, 0, 0)),
> +                      ((0, 0, 0, 0), (4, 0, 0, 0), (4, 0, 0, 0)),
> +                      ((1, 0, 0, 0), (4, 0, 0, 0), (5, 0, 0, 0)),

What the heck is this mysterious matrix?
Comment 3 Timothy Loh 2013-02-03 21:53:46 PST
(In reply to comment #2)
> (From update of attachment 186300 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186300&action=review
> 
> > Tools/Scripts/webkitpy/tool/steps/preparechangelog.py:87
> > -        new_date_line = new_entry.date_line()
> > -        old_date_line = old_entry.date_line()
> > -        if new_date_line != old_date_line:
> > -            final_entry = final_entry.replace(old_date_line, new_date_line)
> > +        final_entry = final_entry.replace(old_entry.date(), new_entry.date(), 1)
> 
> Why are we making this change? This patch needs per-function change log.

I'm not sure what you're asking here.

> > Tools/Scripts/webkitpy/tool/steps/preparechangelog_unittest.py:99
> > +                      ((4, 0, 0, 0), (0, 0, 0, 0), (0, 0, 0, 0)),
> > +                      ((5, 0, 0, 0), (0, 0, 0, 0), (1, 0, 0, 0)),
> > +                      ((0, 0, 0, 0), (4, 0, 0, 0), (4, 0, 0, 0)),
> > +                      ((1, 0, 0, 0), (4, 0, 0, 0), (5, 0, 0, 0)),
> 
> What the heck is this mysterious matrix?

It looks a bit odd without context, but basically each 4-tuple represents a ChangeLog entry and each line is (new entry, old entry, expected result entry).
Comment 4 Ryosuke Niwa 2013-02-03 22:01:39 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 186300 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=186300&action=review
> > 
> > > Tools/Scripts/webkitpy/tool/steps/preparechangelog.py:87
> > > -        new_date_line = new_entry.date_line()
> > > -        old_date_line = old_entry.date_line()
> > > -        if new_date_line != old_date_line:
> > > -            final_entry = final_entry.replace(old_date_line, new_date_line)
> > > +        final_entry = final_entry.replace(old_entry.date(), new_entry.date(), 1)
> > 
> > Why are we making this change? This patch needs per-function change log.
> 
> I'm not sure what you're asking here.

Exactly what I said. It's not obvious why this code change is necessary or desirable to fix the bug. This is why I'm suggesting to add per-function description in your change log so that others can understand why you're making each code change.
Comment 5 Timothy Loh 2013-02-03 22:07:45 PST
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 186300 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=186300&action=review
> > > 
> > > > Tools/Scripts/webkitpy/tool/steps/preparechangelog.py:87
> > > > -        new_date_line = new_entry.date_line()
> > > > -        old_date_line = old_entry.date_line()
> > > > -        if new_date_line != old_date_line:
> > > > -            final_entry = final_entry.replace(old_date_line, new_date_line)
> > > > +        final_entry = final_entry.replace(old_entry.date(), new_entry.date(), 1)
> > > 
> > > Why are we making this change? This patch needs per-function change log.
> > 
> > I'm not sure what you're asking here.
> 
> Exactly what I said. It's not obvious why this code change is necessary or desirable to fix the bug. This is why I'm suggesting to add per-function description in your change log so that others can understand why you're making each code change.

Ah, I was confused because the patch is about change logs so I was trying to interpret the sentence differently.
Comment 6 Timothy Loh 2013-02-03 22:17:47 PST
Created attachment 186306 [details]
Patch
Comment 7 WebKit Review Bot 2013-02-04 16:31:56 PST
Comment on attachment 186306 [details]
Patch

Clearing flags on attachment: 186306

Committed r141829: <http://trac.webkit.org/changeset/141829>
Comment 8 WebKit Review Bot 2013-02-04 16:32:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Julien Chaffraix 2013-02-04 16:47:34 PST
FWIW I was looking into who 'regressed' webkit-patch upload today as automatically regenerating the *whole* ChangeLog more or less every time you touch something without a way of opt-out was annoying me.

I shot myself in the foot because of this behavior and don't see a good way to prevent it from happening regularly unless opting out (merging the new entries after the old ones makes it hard to see them IMO). See http://trac.webkit.org/changeset/141810/trunk/Source/WebCore/ChangeLog where the ChangeLog landed duplicated because I didn't pay close attention.