[webkit-dev] ChangeLogs

David Levin levin at chromium.org
Wed Mar 21 14:41:58 PDT 2012

I think the challenge in part is to explain why the ChangeLog is useful.

Comments in there hopefully explain why as a guide to reviewers to give the
reviewers and future onlookers a guide to the change.

It feels like what comments are that useful but often get inserted in there
to fill it out. Is there a good way to address this? (Should folks just
leave what comments out or put in the obvious what?)

Since we have an example ChangeLog, I'll point out what I mean using it
(but it is evident in many ChangeLog's from me as well).

What value does this comment add?

   - runtime/JSGlobalData.h: Replaced the HashSet and HashCountedSet with a

On the other hand, I found this very informative:

(WTF::PassRefPtr::~PassRefPtr): The most common thing to do with a
PassRefPtr in hot code is to pass it and then destroy it once it's
set to zero. Help the optimizer by telling it that's true.

Here again, we see a what comment:

(JSC::NodeReleaser::releaseAllNodes): ALWAYS_INLINE.
(JSC::NodeReleaser::adopt): Ditto.

Which doesn't tell me why ALWAYS_INLINE was added (which I can easily see
from the patch).


On Wed, Mar 21, 2012 at 2:33 PM, Maciej Stachowiak <mjs at apple.com> wrote:

> On Mar 21, 2012, at 2:29 PM, Timothy Hatcher wrote:
> Lately I have observed more <http://trac.webkit.org/changeset/111595> and
> more <http://trac.webkit.org/changeset/111577> and more<http://trac.webkit.org/changeset/111527> changes
> going into WebKit that lack any details about why a particular change was
> made. It is intended that the ChangeLog (and commit message) contain some
> details about your change, not just the bug title and URL.
> The contributing information<http://www.webkit.org/coding/contributing.html#changelogs> on
> subject is pretty sparse, which has likely caused this problem to manifest.
> However, the example ChangeLog <http://trac.webkit.org/changeset/43259> linked
> from that page is prime example of what we all should strive for when
> describing our changes.
> To help curb this lack of detail in ChangeLogs, I propose we add script<https://bugs.webkit.org/show_bug.cgi?id=81828> (or
> augment an existing script) to check for this missing information and
> inform the contributor. It is clear not all reviewers are asking patch
> authors to provide this information when reviewing, and such a tool would
> help enforce it.
> I think this is a reasonable suggestion. I think we should make the
> contributor docs more clear about what is expected, as well.
> Regards,
> Maciej
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120321/42e26009/attachment.html>

More information about the webkit-dev mailing list