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.
Created attachment 148535 [details] Patch
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 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.
Created attachment 148537 [details] Patch 2 (OOPS!).
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 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.
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.
(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 :)
Well, everything here could be addressed in reviews - the purpose of the patch is to suggest a better template.
How about changing the details line to: Optional additional information of the change such as approach, rationale (OOPS!). ?
(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?
Created attachment 149917 [details] Patch 3
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 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
Landed: http://trac.webkit.org/changeset/122076
(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
Created attachment 151188 [details] Patch 4
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 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 on attachment 151188 [details] Patch 4 ok.
Comment on attachment 151188 [details] Patch 4 Clearing flags on attachment: 151188 Committed r122192: <http://trac.webkit.org/changeset/122192>
All reviewed patches have been landed. Closing bug.