Bug 89560 - Improve the boilerplate generated by prepare-ChangeLog
Summary: Improve the boilerplate generated by prepare-ChangeLog
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: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-20 04:18 PDT by Kent Tamura
Modified: 2012-07-09 21:35 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.13 KB, patch)
2012-06-20 04:21 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (2.49 KB, patch)
2012-06-20 04:29 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 3 (2.67 KB, patch)
2012-06-28 04:02 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 4 (9.81 KB, patch)
2012-07-08 23:24 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-06-20 04:18:39 PDT
We had better produce something like:

2012-06-20  Kent Tamura  <tkent@chromium.org>

        Need a short description (OOPS!)
        Need the bug URL (OOPS!)

        Reviewed by NOBODY (OOPS!).

        The detail of the change (OOPS!)

        No new tests. (OOPS!)


to show where the detail should be in.
Comment 1 Kent Tamura 2012-06-20 04:21:13 PDT
Created attachment 148535 [details]
Patch
Comment 2 Kentaro Hara 2012-06-20 04:23:29 PDT
Comment on attachment 148535 [details]
Patch

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

> Tools/ChangeLog:19
> +        >         Need the bug URL (OOPS!)
> +        >
> +        >         Reviewed by NOBODY (OOPS!).
> +        >
> +        >         The detail of the change (OOPS!)
> +        >
> +        >         No new tests. (OOPS!)

Nit: Shall we make the position of '.' consistent?
Comment 3 Kent Tamura 2012-06-20 04:26:38 PDT
Comment on attachment 148535 [details]
Patch

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

>> Tools/ChangeLog:19
>> +        >         No new tests. (OOPS!)
> 
> Nit: Shall we make the position of '.' consistent?

Sounds reasonable.  Maybe we had better make them "... (OOPS!)." because the "Reviewed by" line is parsed by other scripts.
Comment 4 Kent Tamura 2012-06-20 04:29:27 PDT
Created attachment 148537 [details]
Patch 2

(OOPS!).
Comment 5 Kentaro Hara 2012-06-20 04:37:01 PDT
Comment on attachment 148537 [details]
Patch 2

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

> Tools/ChangeLog:17
> +        >         The detail of the change (OOPS!).

'Details of the change' might be a better word (according to my friend:-)
Comment 6 Kent Tamura 2012-06-20 05:08:49 PDT
Comment on attachment 148537 [details]
Patch 2

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

>> Tools/ChangeLog:17
>> +        >         The detail of the change (OOPS!).
> 
> 'Details of the change' might be a better word (according to my friend:-)

Thanks! I'll update it.
Comment 7 Alexey Proskuryakov 2012-06-20 10:25:07 PDT
Details of the change should generally be per-function below. Only in rare cases of sweeping changes it's OK to have a single description at the top.

It looks like this change sends a wrong signal.
Comment 8 Ryosuke Niwa 2012-06-20 11:40:18 PDT
(In reply to comment #7)
> Details of the change should generally be per-function below. Only in rare cases of sweeping changes it's OK to have a single description at the top.
> 
> It looks like this change sends a wrong signal.

That could be addressed in reviews :)
Comment 9 Alexey Proskuryakov 2012-06-20 12:16:17 PDT
Well, everything here could be addressed in reviews - the purpose of the patch is to suggest a better template.
Comment 10 Kent Tamura 2012-06-20 12:57:34 PDT
How about changing the details line to:
        Optional additional information of the change such as approach, rationale (OOPS!).
?
Comment 11 Ryosuke Niwa 2012-06-20 14:16:58 PDT
(In reply to comment #10)
> How about changing the details line to:
>         Optional additional information of the change such as approach, rationale (OOPS!).
> ?

Maybe something like "Please add per-function descriptions if applicable" below the original line?
Comment 12 Kent Tamura 2012-06-28 04:02:26 PDT
Created attachment 149917 [details]
Patch 3
Comment 13 Kentaro Hara 2012-06-28 04:05:09 PDT
Comment on attachment 149917 [details]
Patch 3

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

ap is on vacation, and would like to delegate the review to rniwa.

> Tools/ChangeLog:17
> +        >         Additional information of the change such as approach, rationale. Please add per-function descriptions below. (OOPS!).

Nit: 'below.' => 'below'
Comment 14 WebKit Review Bot 2012-07-08 20:08:05 PDT
Comment on attachment 149917 [details]
Patch 3

Rejecting attachment 149917 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 11988 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
47>At revision 11988.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13170020
Comment 15 Kent Tamura 2012-07-08 20:39:31 PDT
Landed: http://trac.webkit.org/changeset/122076
Comment 16 Kent Tamura 2012-07-08 22:01:59 PDT
(In reply to comment #15)
> Landed: http://trac.webkit.org/changeset/122076

It seems this broke "sheriffbot rollout".  I rolled it out by http://trac.webkit.org/changeset/122084
Comment 17 Kent Tamura 2012-07-08 23:24:35 PDT
Created attachment 151188 [details]
Patch 4
Comment 18 Ryosuke Niwa 2012-07-08 23:30:21 PDT
Comment on attachment 151188 [details]
Patch 4

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

> Tools/ChangeLog:12
> +        >         Need a short description (Oops!).

Can we instead say that need a bug title or bug summary?
Comment 19 Kent Tamura 2012-07-09 18:16:23 PDT
Comment on attachment 151188 [details]
Patch 4

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

>> Tools/ChangeLog:12
>> +        >         Need a short description (Oops!).
> 
> Can we instead say that need a bug title or bug summary?

I'd like to keep this as is because we sometimes don't use the bug title (e.g. security bugs) and "Need a short description, it typically matches to the bug title (OOPS!)." is long.
Comment 20 Ryosuke Niwa 2012-07-09 21:20:08 PDT
Comment on attachment 151188 [details]
Patch 4

ok.
Comment 21 WebKit Review Bot 2012-07-09 21:35:26 PDT
Comment on attachment 151188 [details]
Patch 4

Clearing flags on attachment: 151188

Committed r122192: <http://trac.webkit.org/changeset/122192>
Comment 22 WebKit Review Bot 2012-07-09 21:35:31 PDT
All reviewed patches have been landed.  Closing bug.