[webkit-dev] ChangeLogs

[David Levin]

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
   Vector.

On the other hand, I found this very informative:

wtf/PassRefPtr.h:
(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:

parser/Nodes.cpp:
(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).

dave

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>