Bug 89560 – Improve the boilerplate generated by prepare-ChangeLog

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kent Tamura]

We had better produce something like:

2012-06-20  Kent Tamura  <[e-mail]>

        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.

[Kent Tamura]

Created attachment 148535 [details]
Patch

[Kentaro Hara]

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?

[Kent Tamura]

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.

[Kent Tamura]

Created attachment 148537 [details]
Patch 2

(OOPS!).

[Kentaro Hara]

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:-)

[Kent Tamura]

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.

[Alexey Proskuryakov]

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.

[Ryosuke Niwa]

(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 :)

[Alexey Proskuryakov]

Well, everything here could be addressed in reviews - the purpose of the patch is to suggest a better template.

[Kent Tamura]

How about changing the details line to:
        Optional additional information of the change such as approach, rationale (OOPS!).
?

[Ryosuke Niwa]

(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?

[Kent Tamura]

Created attachment 149917 [details]
Patch 3

[Kentaro Hara]

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'

[WebKit Review Bot]

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

[Kent Tamura]

Landed: http://trac.webkit.org/changeset/122076

[Kent Tamura]

(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

[Kent Tamura]

Created attachment 151188 [details]
Patch 4

[Ryosuke Niwa]

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?

[Kent Tamura]

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.

[Ryosuke Niwa]

Comment on attachment 151188 [details]
Patch 4

ok.

[WebKit Review Bot]

Comment on attachment 151188 [details]
Patch 4

Clearing flags on attachment: 151188

Committed r122192: <http://trac.webkit.org/changeset/122192>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.