WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
108788
Don't update author info in PrepareChangeLog and allow users to skip the PrepareChangeLog step entirely.
https://bugs.webkit.org/show_bug.cgi?id=108788
Summary
Don't update author info in PrepareChangeLog and allow users to skip the Prep...
Timothy Loh
Reported
2013-02-03 21:28:07 PST
Don't update author info in PrepareChangeLog and allow users to skip the PrepareChangeLog step entirely.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Timothy Loh
Comment 1
2013-02-03 21:43:37 PST
Created
attachment 186300
[details]
Patch
Ryosuke Niwa
Comment 2
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?
Timothy Loh
Comment 3
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).
Ryosuke Niwa
Comment 4
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.
Timothy Loh
Comment 5
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.
Timothy Loh
Comment 6
2013-02-03 22:17:47 PST
Created
attachment 186306
[details]
Patch
WebKit Review Bot
Comment 7
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
>
WebKit Review Bot
Comment 8
2013-02-04 16:32:00 PST
All reviewed patches have been landed. Closing bug.
Julien Chaffraix
Comment 9
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug