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 <m...@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@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
