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:
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.
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev