I'm OK with not having the function list. I can see the advantages of the
webkit approach as it means that comments in the code don't get outdated.
But, then it forces you to look at the changelog history of every line of
code.
Anyways, how about we prepopulate gcl/git-cl change with this text:
DETAILED_DESCRIPTION_HERE

BUG=http://bugs.chromium.org/BUG_NUMBER_HERE
TEST=required
RELEASE_NOTES=optional


On Wed, Jul 22, 2009 at 1:28 PM, Jeremy Orlow <jor...@chromium.org> wrote:

> On Wed, Jul 22, 2009 at 1:22 PM, Peter Kasting <pkast...@google.com>wrote:
>
>> On Wed, Jul 22, 2009 at 1:17 PM, Ojan Vafai <o...@chromium.org> wrote:
>>
>>>
>>>>> platform/graphics/chromium/FontPlatformDataLinux.cpp:
>>>>>
>>>>    (WebCore::FontPlatformData::setHinting):
>>>>>   (WebCore::FontPlatformData::setAntiAlias):
>>>>>   (WebCore::FontPlatformData::setSubpixelGlyphs):
>>>>>   (WebCore::FontPlatformData::setupPaint): Modified to do something
>>>>> super special.
>>>>> platform/graphics/chromium/FontPlatformDataLinux.h:
>>>>>
>>>>
>>>> Is it worth putting this into the change list?  If so, maybe 'gcl
>>>> commit' can do it (since doing it any earlier risks it getting stale as the
>>>> CL changes).
>>>>
>>>
>>> The benefit of this is that you can give more detailed comments inline.
>>> You see some WebKit commits do this (e.g. Darin Adler's) and it makes it
>>> much more clear from reading the change description what happened. So, if
>>> we're going to do it, it's not useful to do at commit time. It would be
>>> great if people got in the habit of writing detailed descriptions like this.
>>>
>>
>> I really, really hate this style.
>>
>> I have never encountered a case where a combination of comments-in-code
>> and good-change-description are not both sufficient, and far more readable
>> than the above style.
>>
>> Here's an example of a Darin Adler commit where this leads to a nice clear
>>> and detailed description.
>>>
>>> WebCore:
>>>
>>> 2009-07-15 Darin Adler < da...@apple.com <da...@apple.com>>
>>>
>>> Reviewed by John Sullivan.
>>>
>>> After double-clicking a word, using Shift-arrow to select behaves
>>> unpredictably
>>>  
>>> https://bugs.webkit.org/show_bug.cgi?id=27177<https://bugs.webkit.org/show_bug.cgi?id=27177>
>>>  rdar://problem/7034324
>>>
>>> Test: editing/selection/extend-selection-after-double-click.html
>>>
>>> The bug was due to the m_lastChangeWasHorizontalExtension flag, which was
>>> not
>>> being cleared in many cases where it should have been.
>>>
>>>
>>>    - editing/SelectionController.cpp:
>>>    (WebCore::SelectionController::setSelection): Set
>>>    m_lastChangeWasHorizontalExtension to false. This catches all sorts of 
>>> cases
>>>    that don't flow through the modify function. Before, the flag would 
>>> reflect
>>>    the last call to the modify function, which was not necessarily the last
>>>    selection change.
>>>
>>> This kind of thing should be a comment in the code, and the fact that
>> WebKit does not generally comment code is much to their detriment.
>>
>
> Yes.  For some reason, WebKit likes to replace comments in the code with
> comments in the ChangeLog which seems insane to me.  If you need comments
> this detailed in the ChangeLog then something is wrong with the code IMHO.
>
> I think there are very few cases where you need down to the function level
> of detail.  In most case, you should be putting comments in the code or
> splitting your CL into multiple that each address one specific issue.
>
>>
>>>    - editing/selection/extend-selection-after-double-click.html: Copied
>>>    from LayoutTests/editing/selection/word-granularity.html. Then turned it
>>>    into a new test.
>>>
>>> This kind of thing, if really important, should be a comment in the
>> change description.
>>
>> PK
>>
>
>

--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
    http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to