On Jul 2, 2009, at 9:14 AM, David Kilzer wrote:

On Wednesday, July 1, 2009 10:44:16 PM, Eric Seidel <e...@webkit.org> wrote:

> Results in:
> 2009-07-01  Eric Seidel  <e...@webkit.org>
>
>         Reviewed by NOBODY (OOPS!).
>
> prepare-ChangeLog should have a --bug= argument and use it for
url autofill
>         https://bugs.webkit.org/show_bug.cgi?id=26383
>
>         DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE:
> http://webkit.org/coding/contributing.html FOR MORE INFORMATION
>
>         Tests: fast/foo.html
>
>         * foo.cpp: Added.

- I prefer having "Bug NNNNN: " before the actual bug description so I don't have to parse the URL myself for the bug number. It also acts as a visual marker when I read the ChangeLog entry.

- I like putting angle brackets "<>" around the URL to set it off visually from other text.

- I generally move the "Reviewed by" line after the bug number/ description/URL. When you're reading a ChangeLog entry/commit message (especially an older one), it's generally much more interesting which bug is being fixed rather than knowing who reviewed it. (Also, putting the bug description first makes git's one-line description of each commit much more useful than either having a list of dates and the person who wrote the patch or having a list of patch reviewers.)

I agree with Dave on all three points. git users gotta stick together.

Here's an example entry that follows the format that I (and I think Dave) like:

+2009-04-20  Adam Roben  <aro...@apple.com>
+
+        Change MemoryStream::createInstance to return a COMPtr
+
+ Part of Bug 25294: All WebKit/win classes should return COMPtrs from
+        their static constructor members
+        <https://bugs.webkit.org/show_bug.cgi?id=25294>
+
+        Reviewed by Darin Adler.
+
+        * MemoryStream.cpp:
+        (MemoryStream::createInstance): Changed to return a COMPtr.
+        (MemoryStream::Clone): Updated for createInstance change.
+        * MemoryStream.h: Changed createInstance to return a COMPtr.
+
+        * WebArchive.cpp:
+        (WebArchive::data):
+        * WebCoreSupport/EmbeddedWidget.cpp:
+        (EmbeddedWidget::didReceiveData):
+        * WebDataSource.cpp:
+        (WebDataSource::data):
+        * WebHistory.cpp:
+        (WebHistory::data):
+        * WebIconFetcher.cpp:
+        (WebIconFetcherClient::finishedFetchingIcon):
+        * WebResource.cpp:
+        (WebResource::createInstance):
+        Updated for changes to MemoryStream::createInstance.
+


-Adam

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Reply via email to